Static and top-level methods can be compile time constants.

Static and top-level methods can be compile time constants.
Committed: https://code.google.com/p/dart/source/detail?r=11387

[kasperl, 2012-08-27 13:17:26]

LGTM.

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
File lib/compiler/implementation/compile_time_constants.dart (right):

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
lib/compiler/implementation/compile_time_constants.dart:37: class
FunctionConstant extends Constant{
Space between Constant and {.

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
lib/compiler/implementation/compile_time_constants.dart:55: void
_writeCanonicalizedJsCode(CodeBuffer buffer, ConstantHandler handler) {
Don't you need to also provide an implementation of _writeJsCode? It's abstract
in Constant.

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
lib/compiler/implementation/compile_time_constants.dart:875:
compiler.codegenWorld.staticFunctionsNeedingGetter.add(element);
Could this registration happen later in the codegen? It feels a bit nasty to
have the registration in here -- I'd rather have it in codegen where we do tons
of registration already.

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
File lib/compiler/implementation/ssa/nodes.dart (right):

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
lib/compiler/implementation/ssa/nodes.dart:165: if (constant.isFunction())
return HType.UNKNOWN;
I guess it could be represented as a HBoundType using the Function class, but
maybe this is fine for now.

https://chromiumcodereview.appspot.com/10872059/diff/7001/tests/language/meth...
File tests/language/methods_as_constants_test.dart (right):

https://chromiumcodereview.appspot.com/10872059/diff/7001/tests/language/meth...
tests/language/methods_as_constants_test.dart:33: Expect.equals('tstsstss',
'$buffer');
This would probably be easier to maintain if you checked the individual runs
rather than the entire sequence. Maybe add another helper:

   Expect.equals("t", run(const A(topLevelMethod)));

and have run reset the buffer and return the buffer as a string? It would also
be nice with tests that you can use initialize a static const / top-level const
with a reference to a static / top-level method (4 combinations).

[ngeoffray, 2012-08-27 14:09:09]

Thank you Kasper

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
File lib/compiler/implementation/compile_time_constants.dart (right):

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
lib/compiler/implementation/compile_time_constants.dart:37: class
FunctionConstant extends Constant{
On 2012/08/27 13:17:26, kasperl wrote:
> Space between Constant and {.

Done.

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
lib/compiler/implementation/compile_time_constants.dart:55: void
_writeCanonicalizedJsCode(CodeBuffer buffer, ConstantHandler handler) {
On 2012/08/27 13:17:26, kasperl wrote:
> Don't you need to also provide an implementation of _writeJsCode? It's
abstract
> in Constant.

Yes, but it will never be called because there's nothing to write for functions
(this method is used for the definition of a CTC, and a function is already
defined). I made it throw an internalError.

https://chromiumcodereview.appspot.com/10872059/diff/7001/lib/compiler/implem...
lib/compiler/implementation/compile_time_constants.dart:875:
compiler.codegenWorld.staticFunctionsNeedingGetter.add(element);
On 2012/08/27 13:17:26, kasperl wrote:
> Could this registration happen later in the codegen? It feels a bit nasty to
> have the registration in here -- I'd rather have it in codegen where we do
tons
> of registration already.

It has to be done here, the codegen only handles uses of the method in a method
body. For fields, the CTC handler is the codegen.

https://chromiumcodereview.appspot.com/10872059/diff/7001/tests/language/meth...
File tests/language/methods_as_constants_test.dart (right):

https://chromiumcodereview.appspot.com/10872059/diff/7001/tests/language/meth...
tests/language/methods_as_constants_test.dart:33: Expect.equals('tstsstss',
'$buffer');
On 2012/08/27 13:17:26, kasperl wrote:
> This would probably be easier to maintain if you checked the individual runs
> rather than the entire sequence. Maybe add another helper:
> 
>    Expect.equals("t", run(const A(topLevelMethod)));
> 
> and have run reset the buffer and return the buffer as a string? It would also
> be nice with tests that you can use initialize a static const / top-level
const
> with a reference to a static / top-level method (4 combinations).

Done.

[ahe, 2012-08-28 13:35:54]

FYI

https://chromiumcodereview.appspot.com/10872059/diff/11001/lib/compiler/imple...
File lib/compiler/implementation/compile_time_constants.dart (right):

https://chromiumcodereview.appspot.com/10872059/diff/11001/lib/compiler/imple...
lib/compiler/implementation/compile_time_constants.dart:64: int hashCode() =>
element.hashCode();
You should make sure that FunctionConstant has a different hashCode from
element. It's good practice, a not-come-back-and-bite-you thing.