In constant evaluation, properly handle redirecting factory constructors.

In constant evaluation, properly handle redirecting factory constructors.

In order to unit test that this change doesn't regress the handling of
"const Symbol(...)", it was necessary to expand TestTypeProvider's
Symbol class to include the Symbol constructor.

Note: with this fix, analyzer detected a previously unknown flaw in
tests/language/switch_case_test.dart: class D didn't implement a
getter for x.  Test updated accordingly.

This change breaks test lib/_internal/compiler/samples/jsonify/jsonify
by changing analyzer's handling of bool.fromEnvironment().  I plan to
address this in a future changelist.

BUG=dartbug.com/17209
R=brianwilkerson@google.com, floitsch@google.com

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

[Brian Wilkerson, 2014-05-06 21:44:31]

LGTM

https://codereview.chromium.org/261403004/diff/1/editor/tools/plugins/com.goo...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/constant/ConstantVisitor.java
(right):

https://codereview.chromium.org/261403004/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/constant/ConstantVisitor.java:299:
// constructor to delegate to, we can't evaluate the constant.
That's sort of true (if you add "currently"). Actually the analysis context
should have a map from defined names to values and we should look in the map
before defaulting to the behavior you've implemented. Then the command-line
analyzer should support -D flags by populating the map.

https://codereview.chromium.org/261403004/diff/1/tests/language/switch_case_t...
File tests/language/switch_case_test.dart (right):

https://codereview.chromium.org/261403004/diff/1/tests/language/switch_case_t...
tests/language/switch_case_test.dart:27: int get x => 0;
I don't know enough about this test to know whether the absence of this line was
significant (although I admit it seems unlikely in this case). We should
probably get the author to review the change.

[Paul Berry, 2014-05-06 21:49:32]

Florian, does the change to tests/language/switch_case_test.dart look correct to
you?

[floitsch, 2014-05-07 09:16:53]

test change LGTM.

[Paul Berry, 2014-05-07 16:27:10]

Committed patchset #1 manually as r35853 (presubmit successful).

[Paul Berry, 2014-05-07 17:21:42]

https://codereview.chromium.org/261403004/diff/1/editor/tools/plugins/com.goo...
File
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/constant/ConstantVisitor.java
(right):

https://codereview.chromium.org/261403004/diff/1/editor/tools/plugins/com.goo...
editor/tools/plugins/com.google.dart.engine/src/com/google/dart/engine/internal/constant/ConstantVisitor.java:299:
// constructor to delegate to, we can't evaluate the constant.
On 2014/05/06 21:44:32, Brian Wilkerson wrote:
> That's sort of true (if you add "currently"). Actually the analysis context
> should have a map from defined names to values and we should look in the map
> before defaulting to the behavior you've implemented. Then the command-line
> analyzer should support -D flags by populating the map.

Ok, I added the word "currently", and the following text:

TODO(paulberry): if the constructor is one of
{bool,int,String}.fromEnvironment(), we may be able to infer the value based on
-D flags provided to the analyzer (see dartbug.com/17234).