Move compile-time constant registrations to the backend.

Move compile-time constant registrations to the backend.

This prepares for handling compile-time constant evaluation in the resolution post-processing.

BUG=
R=ahe@google.com, karlklose@google.com, ngeoffray@google.com

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

[Johnni Winther, 2013-09-27 15:14:02]

@ahe: Look at the metadata handling.
@ngeoffray: Look at the backend registration.

[ahe, 2013-09-30 11:05:26]

LGTM except for an excessive amount of extra lines.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/constants.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/constants.dart:299: String toString()
{
Nice with toString implementation, but I suggest that you do something to
distinguish the values from constants. For example:

  return 'StringConstant(${Error.safeToString(value.slowToString())})';

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/constants.dart:300: return "'" +
value.slowToString() + "'";
Use string interpolation, please.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:740: 
Extra line.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:751: DartType
type = constant.computeType(compiler);
Add todo to get rid of Constant.computeType?

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:154: 
Extra line.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/typechecker.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/typechecker.dart:166: return
thisTypeCache;
This isn't a cache, please find a different name.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/util/util.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/util/util.dart:169:
//print(lines.last);
Delete debug code.

[ngeoffray, 2013-10-01 08:33:35]

LGTM

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:127: /// Called by the
backend when [constant] has been used.
Please update comment: how can that be called by the backend, being it a method
on the backend?

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:1800: for
(Dependency dependency in metadataConstants) {
Why is that not covered by registerMetadataConstant?

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:363: //
TODO(karlklose): This should never happen, not even for constructors.
Please add: TODO(...): remove this check. elements being null should never
happen, ...

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:421: if
(!compiler.analyzeSignaturesOnly || tree.isRedirectingFactory) {
What's this redirecting factory check? Please add a comment.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:484: if
(!element.modifiers.isConst()) {
Note that this is not fixing it: what we want to know here is whether the value
on the left hand side is a constant. Can't we eagerly request to compile it?

[karlklose, 2013-10-01 08:58:58]

LGTM.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/typechecker.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/typechecker.dart:198: thisTypeCache =
currentClass.computeType(compiler);
Can you use currentClass.thisType here?

[Johnni Winther, 2013-10-01 11:21:47]

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/compiler.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/compiler.dart:127: /// Called by the
backend when [constant] has been used.
On 2013/10/01 08:33:35, ngeoffray wrote:
> Please update comment: how can that be called by the backend, being it a
method
> on the backend?

Done.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/constants.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/constants.dart:299: String toString()
{
On 2013/09/30 11:05:26, ahe wrote:
> Nice with toString implementation, but I suggest that you do something to
> distinguish the values from constants. For example:
> 
>   return 'StringConstant(${Error.safeToString(value.slowToString())})';

Done.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/constants.dart:300: return "'" +
value.slowToString() + "'";
On 2013/09/30 11:05:26, ahe wrote:
> Use string interpolation, please.

Done.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/js_backend/backend.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:740: 
On 2013/09/30 11:05:26, ahe wrote:
> Extra line.

Done.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:751: DartType
type = constant.computeType(compiler);
On 2013/09/30 11:05:26, ahe wrote:
> Add todo to get rid of Constant.computeType?

Done.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/js_backend/backend.dart:1800: for
(Dependency dependency in metadataConstants) {
On 2013/10/01 08:33:35, ngeoffray wrote:
> Why is that not covered by registerMetadataConstant?

This handles the late registration, when [mustRetainMetadata] has been set to
[:true:] after the constants have been registered with
[registerMetadataConstant].

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/resolution/members.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:154: 
On 2013/09/30 11:05:26, ahe wrote:
> Extra line.

Done.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:363: //
TODO(karlklose): This should never happen, not even for constructors.
On 2013/10/01 08:33:35, ngeoffray wrote:
> Please add: TODO(...): remove this check. elements being null should never
> happen, ...

Done.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:421: if
(!compiler.analyzeSignaturesOnly || tree.isRedirectingFactory) {
On 2013/10/01 08:33:35, ngeoffray wrote:
> What's this redirecting factory check? Please add a comment.

Added comment: We need to analyze the redirecting factory bodies to ensure that
we can analyze compile-time constants.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/resolution/members.dart:484: if
(!element.modifiers.isConst()) {
On 2013/10/01 08:33:35, ngeoffray wrote:
> Note that this is not fixing it: what we want to know here is whether the
value
> on the left hand side is a constant. Can't we eagerly request to compile it?

Added a TODO.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/typechecker.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/typechecker.dart:166: return
thisTypeCache;
On 2013/09/30 11:05:26, ahe wrote:
> This isn't a cache, please find a different name.

Removed the getters.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/typechecker.dart:198: thisTypeCache =
currentClass.computeType(compiler);
On 2013/10/01 08:58:58, karlklose wrote:
> Can you use currentClass.thisType here?

Added a TODO.

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
File sdk/lib/_internal/compiler/implementation/util/util.dart (right):

https://codereview.chromium.org/24282005/diff/9001/sdk/lib/_internal/compiler...
sdk/lib/_internal/compiler/implementation/util/util.dart:169:
//print(lines.last);
On 2013/09/30 11:05:26, ahe wrote:
> Delete debug code.

Done.

[ngeoffray, 2013-10-01 11:27:10]

Still LGTM

[Johnni Winther, 2013-10-01 11:33:18]

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