Stop resolving classes prematurely in the vm (issue 11023).

Stop resolving classes prematurely in the vm (issue 11023).
Add missing class finalization to mirrors tests.

R=hausner@google.com

Committed: https://code.google.com/p/dart/source/detail?r=24965

[hausner, 2013-07-11 22:18:27]

LGTM modulo the discussion in the other thread.

Moving the ambiguous reference check into the library object can be done as a
separate change.

https://codereview.chromium.org/19030004/diff/6001/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

https://codereview.chromium.org/19030004/diff/6001/runtime/vm/class_finalizer...
runtime/vm/class_finalizer.cc:267: // TODO(regis): Call
lib.LookupClass(class_name, ambiguity_error) instead
Maybe you are right and this ambiguity test should be done in the library class,
instead of being duplicated here and in the parser.

https://codereview.chromium.org/19030004/diff/6001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/19030004/diff/6001/runtime/vm/object.cc#newco...
runtime/vm/object.cc:6674: // is found in more than one import and actually
used.
This error check is done in the parser. It verifies that a name is not
ambiguous. See Parser::ResolveNameInCurrentLibraryScope().

[regis, 2013-07-12 00:02:38]

Thanks Matthias.

This updated version takes into account the "modulo the other discussion".

I am still chasing one failing test in checked mode (type_parameter_test/none),
but you can already have another look.

Thanks,
Regis

https://codereview.chromium.org/19030004/diff/6001/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

https://codereview.chromium.org/19030004/diff/6001/runtime/vm/class_finalizer...
runtime/vm/class_finalizer.cc:267: // TODO(regis): Call
lib.LookupClass(class_name, ambiguity_error) instead
On 2013/07/11 22:18:27, hausner wrote:
> Maybe you are right and this ambiguity test should be done in the library
class,
> instead of being duplicated here and in the parser.

Will do in a following cl.

https://codereview.chromium.org/19030004/diff/6001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/19030004/diff/6001/runtime/vm/object.cc#newco...
runtime/vm/object.cc:6674: // is found in more than one import and actually
used.
On 2013/07/11 22:18:27, hausner wrote:
> This error check is done in the parser. It verifies that a name is not
> ambiguous. See Parser::ResolveNameInCurrentLibraryScope().

Understood. My comment relates to the fact that this function cannot be used by
itself, since it does not detect ambiguities. I will rewrite it in another cl.

[regis, 2013-07-12 03:24:37]

I have uploaded one more time. The bug I mentioned pre-existed and was uncovered
by this change. I cc'ed you on the other cl fixing it.

Thanks,
Regis

[hausner, 2013-07-12 05:30:22]

LGTM.

https://codereview.chromium.org/19030004/diff/30001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/19030004/diff/30001/runtime/vm/parser.cc#newc...
runtime/vm/parser.cc:5330: if (type.IsTypeParameter() ||  // Always resolved at
parse time.
At parse time of what? I don't understand the comment.

[regis, 2013-07-12 17:58:55]

Thanks!

https://codereview.chromium.org/19030004/diff/30001/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

https://codereview.chromium.org/19030004/diff/30001/runtime/vm/parser.cc#newc...
runtime/vm/parser.cc:5330: if (type.IsTypeParameter() ||  // Always resolved at
parse time.
On 2013/07/12 05:30:22, hausner wrote:
> At parse time of what? I don't understand the comment.

I clarified the comment.

[regis, 2013-07-12 17:59:24]

Committed patchset #6 manually as r24965 (presubmit successful).