Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(283)

Issue 10689104: Fix const unparse. Example: (Closed)

Created:
8 years, 5 months ago by Roman
Modified:
8 years, 5 months ago
Reviewers:
Anton Muhin
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Fix const unparse. Example: foo([var a = const[]]) {} used to unparse to: foo([var a = []]) {} Committed: https://code.google.com/p/dart/source/detail?r=9439

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : add tests #

Total comments: 2

Patch Set 4 : Address Anton comments #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+14 lines, -2 lines) Patch
M lib/compiler/implementation/tree/unparser.dart View 1 2 2 chunks +6 lines, -2 lines 1 comment Download
M tests/compiler/dart2js/unparser_test.dart View 1 2 3 2 chunks +8 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
Roman
8 years, 5 months ago (2012-07-06 10:14:04 UTC) #1
Anton Muhin
lgtm https://chromiumcodereview.appspot.com/10689104/diff/3001/tests/compiler/dart2js/unparser_test.dart File tests/compiler/dart2js/unparser_test.dart (right): https://chromiumcodereview.appspot.com/10689104/diff/3001/tests/compiler/dart2js/unparser_test.dart#newcode97 tests/compiler/dart2js/unparser_test.dart:97: testConstParameterModifier() { please, add tests for: foo([var a ...
8 years, 5 months ago (2012-07-06 10:22:46 UTC) #2
Roman
8 years, 5 months ago (2012-07-06 11:46:05 UTC) #3
https://chromiumcodereview.appspot.com/10689104/diff/3001/tests/compiler/dart...
File tests/compiler/dart2js/unparser_test.dart (right):

https://chromiumcodereview.appspot.com/10689104/diff/3001/tests/compiler/dart...
tests/compiler/dart2js/unparser_test.dart:97: testConstParameterModifier() {
On 2012/07/06 10:22:46, antonmuhin wrote:
> please, add tests for:
> 
> foo([var a = const {}]) {}
> 
> and for
> 
> foo() {
>   var a = const [];
>   var b = const {};
> }
> 
> bonus point for something like:
> 
> foo([var a = const [ const { 'a': const [ 1, 2, 3 ] } ]]) {}
> 
> And what about const Foo() constructs overall?

Done. And I reverted the case for NewExpression.

https://chromiumcodereview.appspot.com/10689104/diff/5002/lib/compiler/implem...
File lib/compiler/implementation/tree/unparser.dart (right):

https://chromiumcodereview.appspot.com/10689104/diff/5002/lib/compiler/implem...
lib/compiler/implementation/tree/unparser.dart:147: // TODO(ahe): handle
'const'.
I reverted this, NewExpression does not have constKeyword, this case needs more
investigation.

Powered by Google App Engine
This is Rietveld 408576698