Support correct factory syntax in the VM as decribed in the spec.

Support correct factory syntax in the VM as decribed in the spec.
Change the default value of --silent_warnings from true to false, so that
warnings are visible when legacy factory syntax is used.

In order to facilitate transition of libraries and user programs, legacy factory
syntax is still supported by 2 workarounds, but warnings are printed.

The first workaround allows the factory clause to omit the type
parameter list, which otherwise has to match exactly with the list of
type parameters in the factory class declaration.

The second workaround allows a factory method to omit the type
parameter list. In that case, the type parameter list of the enclosing
class is used instead.

Workarounds will be removed to enforce correct factory syntax at a later time.
Committed: https://code.google.com/p/dart/source/detail?r=1595

[srdjan, 2011-11-16 23:02:26]

LGTM with comments & questions

http://codereview.chromium.org/8585004/diff/1/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

http://codereview.chromium.org/8585004/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:359: Class::Handle(ResolveClass(interface,
unresolved_factory_class));
Can the factory_class be null (i.e., non-existing). Please add comment or code.

http://codereview.chromium.org/8585004/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:373: if (num_type_params == 0) {
Do we report warning in correct cases when interface and factory methods both
have no type arguments?

http://codereview.chromium.org/8585004/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:704: }
Why don't you merge the two function.IsFactory() if-blocks and an else with
type=ResolveType....; function.set_result....

http://codereview.chromium.org/8585004/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

http://codereview.chromium.org/8585004/diff/1/runtime/vm/parser.cc#newcode2365
runtime/vm/parser.cc:2365: // constructed instance, which is passed in as an
hidden parameter.
a hidden

http://codereview.chromium.org/8585004/diff/1/runtime/vm/parser.cc#newcode2549
runtime/vm/parser.cc:2549: // constructed instance, which is passed in as an
hidden parameter.
a hidden

http://codereview.chromium.org/8585004/diff/1/tests/language/language.status
File tests/language/language.status (right):

http://codereview.chromium.org/8585004/diff/1/tests/language/language.status#...
tests/language/language.status:88: TypeVariableBoundsTest/none: Fail 	 # Issue
417
A tab?

http://codereview.chromium.org/8585004/diff/1/tests/language/language.status#...
tests/language/language.status:89: TypeVariableScopeTest/none: Fail         #
Issue 417
I think issue 417 should be marked as resolved, there is no crash. The test must
be fixed though.

http://codereview.chromium.org/8585004/diff/1/tests/language/src/FBoundedQuan...
File tests/language/src/FBoundedQuantificationTest.dart (right):

http://codereview.chromium.org/8585004/diff/1/tests/language/src/FBoundedQuan...
tests/language/src/FBoundedQuantificationTest.dart:5: // Test for F-Bounded
Quantification.
Is this file related to factory change?

[regis, 2011-11-16 23:38:08]

Thanks!

http://codereview.chromium.org/8585004/diff/1/runtime/vm/class_finalizer.cc
File runtime/vm/class_finalizer.cc (right):

http://codereview.chromium.org/8585004/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:359: Class::Handle(ResolveClass(interface,
unresolved_factory_class));
On 2011/11/16 23:02:26, srdjan wrote:
> Can the factory_class be null (i.e., non-existing). Please add comment or
code.

Cannot be null. Added assert.

http://codereview.chromium.org/8585004/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:373: if (num_type_params == 0) {
On 2011/11/16 23:02:26, srdjan wrote:
> Do we report warning in correct cases when interface and factory methods both
> have no type arguments?

Good catch! I added the warning at a later stage and did not think of that case.

http://codereview.chromium.org/8585004/diff/1/runtime/vm/class_finalizer.cc#n...
runtime/vm/class_finalizer.cc:704: }
On 2011/11/16 23:02:26, srdjan wrote:
> Why don't you merge the two function.IsFactory() if-blocks and an else with
> type=ResolveType....; function.set_result....

Done.

http://codereview.chromium.org/8585004/diff/1/runtime/vm/parser.cc
File runtime/vm/parser.cc (right):

http://codereview.chromium.org/8585004/diff/1/runtime/vm/parser.cc#newcode2365
runtime/vm/parser.cc:2365: // constructed instance, which is passed in as an
hidden parameter.
On 2011/11/16 23:02:26, srdjan wrote:
> a hidden

Merci :-)

http://codereview.chromium.org/8585004/diff/1/runtime/vm/parser.cc#newcode2549
runtime/vm/parser.cc:2549: // constructed instance, which is passed in as an
hidden parameter.
On 2011/11/16 23:02:26, srdjan wrote:
> a hidden

Done.

http://codereview.chromium.org/8585004/diff/1/tests/language/language.status
File tests/language/language.status (right):

http://codereview.chromium.org/8585004/diff/1/tests/language/language.status#...
tests/language/language.status:88: TypeVariableBoundsTest/none: Fail 	 # Issue
417
On 2011/11/16 23:02:26, srdjan wrote:
> A tab?

Done.

http://codereview.chromium.org/8585004/diff/1/tests/language/language.status#...
tests/language/language.status:89: TypeVariableScopeTest/none: Fail         #
Issue 417
On 2011/11/16 23:02:26, srdjan wrote:
> I think issue 417 should be marked as resolved, there is no crash. The test
must
> be fixed though.

The test is negative and is proper. Note that we are in the dartc section here.
So I will reassign the bug to dartc/frog.

http://codereview.chromium.org/8585004/diff/1/tests/language/src/FBoundedQuan...
File tests/language/src/FBoundedQuantificationTest.dart (right):

http://codereview.chromium.org/8585004/diff/1/tests/language/src/FBoundedQuan...
tests/language/src/FBoundedQuantificationTest.dart:5: // Test for F-Bounded
Quantification.
On 2011/11/16 23:02:26, srdjan wrote:
> Is this file related to factory change?

No, but this test was mixed with upper bounds tests in
tests/language/src/TypeVariableBoundsTest.dart.
It is better to make it a separate test.