Use default values of named arguments when invoking the default super constructor.

Use default values of named arguments when invoking the default super constructor.

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

[ngeoffray, 2012-03-30 09:10:03]

https://chromiumcodereview.appspot.com/9930005/diff/1/lib/compiler/implementa...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9930005/diff/1/lib/compiler/implementa...
lib/compiler/implementation/ssa/builder.dart:1766: }
I think the creation of the synthethized node, and adding the selector in the
TreeElements should be done in the resolver. 

Likewise, the resolver should report an error if it cannot find a default
constructor to call. I just tried this code:

class A {
  A(a);
}

class B extends A {
  B(c);
}

main() {
  print(new B(1));
}

We report:
foo.dart:2:5: internal error: Cannot find value parameter(a)
  A(a);
    ^
We should clearly report an error before entering codegen. So if the resolver
does the check, it should also add the synthethized send.

[floitsch, 2012-03-30 18:18:28]

https://chromiumcodereview.appspot.com/9930005/diff/1/lib/compiler/implementa...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9930005/diff/1/lib/compiler/implementa...
lib/compiler/implementation/ssa/builder.dart:1766: }
On 2012/03/30 09:10:03, ngeoffray wrote:
> I think the creation of the synthethized node, and adding the selector in the
> TreeElements should be done in the resolver. 
Not sure. (I changed this line from "I disagree", to "I think I disagree", to
"Not sure."...).

> 
> Likewise, the resolver should report an error if it cannot find a default
> constructor to call. I just tried this code:
Definitely.

> 
> class A {
>   A(a);
> }
> 
> class B extends A {
>   B(c);
> }
> 
> main() {
>   print(new B(1));
> }
> 
> We report:
> foo.dart:2:5: internal error: Cannot find value parameter(a)
>   A(a);
>     ^
> We should clearly report an error before entering codegen. So if the resolver
> does the check, it should also add the synthethized send.
It would definitely make life easier (both in the CTC as well as in the
builder), but somehow it looks to me as if the synthetic send is an
implementation detail.
That said: we already create a synthetic constructor too. The main-difference
(from my point-of-view), is that the synthetic constructor is a data-structure
that comes from the resolver. Whereas the synthetic send instantiates something
from the Parser.

[floitsch, 2012-04-11 14:53:21]

Making karl the main-reviewer since Nicolas is on vacation.

[karlklose, 2012-04-12 13:33:51]

LGTM.

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
File lib/compiler/implementation/compile_time_constants.dart (right):

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/compile_time_constants.dart:952: * Retruns [:null:]
if the [target] does not apply to the selector.
Retruns -> returns.

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/compile_time_constants.dart:988:
List<Constant>jsNewArguments = evaluator.buildJsNewArguments(classElement);
Not your code, but please add a space between variable type and name, even
though it makes the line exceeds the limit.

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/compile_time_constants.dart:1059: if (arguments ===
null) return false;
arguments -> compiledArguments? Remove?

[ngeoffray, 2012-04-16 12:24:01]

LGTM, with Karl's comments

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/ssa/builder.dart:818: assert(succeeded);
Please add a comment why this is an assert and not an internalError.

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/ssa/builder.dart:890: compiler.internalError("no
default constructor available");
Could you also update the resolver to report the 'real' error?

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
File lib/compiler/implementation/universe.dart (right):

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/universe.dart:135:
addMatchingSendArgumentsToList(arguments);
addMatchingSendArgumentsToList -> addMatchingArgumentsToList?

[floitsch, 2012-04-16 14:23:14]

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
File lib/compiler/implementation/compile_time_constants.dart (right):

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/compile_time_constants.dart:952: * Retruns [:null:]
if the [target] does not apply to the selector.
On 2012/04/12 13:33:51, karlklose wrote:
> Retruns -> returns.

As discussed now asserts that it succeeded. So never returns null.

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/compile_time_constants.dart:988:
List<Constant>jsNewArguments = evaluator.buildJsNewArguments(classElement);
On 2012/04/12 13:33:51, karlklose wrote:
> Not your code, but please add a space between variable type and name, even
> though it makes the line exceeds the limit. 

Done.

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/compile_time_constants.dart:1059: if (arguments ===
null) return false;
On 2012/04/12 13:33:51, karlklose wrote:
> arguments -> compiledArguments? Remove?

removed.

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
File lib/compiler/implementation/ssa/builder.dart (right):

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/ssa/builder.dart:818: assert(succeeded);
On 2012/04/16 12:24:02, ngeoffray wrote:
> Please add a comment why this is an assert and not an internalError.

changed to internal error.

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/ssa/builder.dart:890: compiler.internalError("no
default constructor available");
On 2012/04/16 12:24:02, ngeoffray wrote:
> Could you also update the resolver to report the 'real' error?

The resolver does the right thing now.

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
File lib/compiler/implementation/universe.dart (right):

https://chromiumcodereview.appspot.com/9930005/diff/5002/lib/compiler/impleme...
lib/compiler/implementation/universe.dart:135:
addMatchingSendArgumentsToList(arguments);
On 2012/04/16 12:24:02, ngeoffray wrote:
> addMatchingSendArgumentsToList -> addMatchingArgumentsToList?

Inlined the function.