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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 10824062: dart2dart (Closed)

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

Description

dart2dart Several fixes for more robust renaming of type annotations and Send-s: Correct renaming of class interfaces with type arguments, renaming of global functions, constructor and named constructor calls, static field accesses. Committed: https://code.google.com/p/dart/source/detail?r=10003

Patch Set 1 #

Patch Set 2 : #

Total comments: 8

Patch Set 3 : #

Patch Set 4 : Updated test statuses #

Total comments: 2

Patch Set 5 : #

Total comments: 3
Unified diffs Side-by-side diffs Delta from patch set Stats (+207 lines, -448 lines) Patch
M lib/compiler/implementation/dart_backend/emitter.dart View 1 chunk +1 line, -0 lines 0 comments Download
M lib/compiler/implementation/dart_backend/renamer.dart View 1 2 3 chunks +29 lines, -29 lines 0 comments Download
M lib/compiler/implementation/resolver.dart View 1 2 3 chunks +31 lines, -8 lines 3 comments Download
M tests/compiler/dart2js/unparser_test.dart View 1 3 chunks +120 lines, -1 line 0 comments Download
M tests/corelib/corelib.status View 1 2 3 1 chunk +3 lines, -67 lines 0 comments Download
M tests/language/language.status View 1 2 3 4 chunks +18 lines, -331 lines 0 comments Download
M tests/language/language_dart2js.status View 1 2 3 3 chunks +5 lines, -1 line 0 comments Download
M tests/lib/lib.status View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download
M tests/standalone/standalone.status View 1 2 3 4 4 chunks +0 lines, -9 lines 0 comments Download
M tests/utils/utils.status View 1 2 3 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 9 (0 generated)
Roman
8 years, 4 months ago (2012-07-27 09:38:17 UTC) #1
Anton Muhin
pretty neat https://chromiumcodereview.appspot.com/10824062/diff/2001/lib/compiler/implementation/dart_backend/renamer.dart File lib/compiler/implementation/dart_backend/renamer.dart (right): https://chromiumcodereview.appspot.com/10824062/diff/2001/lib/compiler/implementation/dart_backend/renamer.dart#newcode28 lib/compiler/implementation/dart_backend/renamer.dart:28: String getFactoryName(FunctionExpression node) { nit: arrow syntax? ...
8 years, 4 months ago (2012-07-27 11:15:34 UTC) #2
Roman
https://chromiumcodereview.appspot.com/10824062/diff/2001/lib/compiler/implementation/dart_backend/renamer.dart File lib/compiler/implementation/dart_backend/renamer.dart (right): https://chromiumcodereview.appspot.com/10824062/diff/2001/lib/compiler/implementation/dart_backend/renamer.dart#newcode28 lib/compiler/implementation/dart_backend/renamer.dart:28: String getFactoryName(FunctionExpression node) { On 2012/07/27 11:15:34, Anton Mukhin ...
8 years, 4 months ago (2012-07-27 11:32:45 UTC) #3
Anton Muhin
lgtm! Please, run -cdart2dart tests.
8 years, 4 months ago (2012-07-27 11:33:50 UTC) #4
Roman
Updated test statuses. Some new test failed, but many tests now pass.
8 years, 4 months ago (2012-07-27 14:22:26 UTC) #5
Anton Muhin
STV! https://chromiumcodereview.appspot.com/10824062/diff/2002/tests/standalone/standalone.status File tests/standalone/standalone.status (right): https://chromiumcodereview.appspot.com/10824062/diff/2002/tests/standalone/standalone.status#newcode163 tests/standalone/standalone.status:163: nit: please, remove
8 years, 4 months ago (2012-07-27 14:25:00 UTC) #6
Roman
https://chromiumcodereview.appspot.com/10824062/diff/2002/tests/standalone/standalone.status File tests/standalone/standalone.status (right): https://chromiumcodereview.appspot.com/10824062/diff/2002/tests/standalone/standalone.status#newcode163 tests/standalone/standalone.status:163: On 2012/07/27 14:25:00, Anton Mukhin wrote: > nit: please, ...
8 years, 4 months ago (2012-07-27 15:02:08 UTC) #7
ahe
It would be a big help if you could revert the changes to resolver.dart. They ...
8 years, 4 months ago (2012-08-02 17:50:17 UTC) #8
ahe
8 years, 4 months ago (2012-08-02 18:56:15 UTC) #9 
A few comments to help Roman better understand the dangerous parts of
resolution.

https://chromiumcodereview.appspot.com/10824062/diff/1006/lib/compiler/implem...
File lib/compiler/implementation/resolver.dart (left):

https://chromiumcodereview.appspot.com/10824062/diff/1006/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:772: Type argType =
resolveTypeAnnotationInContext(context,
The original code is probably broken as well and may cause infinite loops.

https://chromiumcodereview.appspot.com/10824062/diff/1006/lib/compiler/implem...
File lib/compiler/implementation/resolver.dart (right):

https://chromiumcodereview.appspot.com/10824062/diff/1006/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:756: type =
element.computeType(compiler);
I cannot rule out that this won't lead to an infinite loop.

https://chromiumcodereview.appspot.com/10824062/diff/1006/lib/compiler/implem...
lib/compiler/implementation/resolver.dart:789: // TypeVariable may happen in
default clause of an interface.
This is a hack. The problem is that the default clause should not be resolved
through this code.

Powered by Google App Engine
This is Rietveld 408576698