Skip to content

Commit 32f3c78

Browse files
author
John Messerly
committed
fix temps that have the same name to have different Elements
this was happening because we picked up an == operator from analyzer's Element also adds an assert, because I took way too much time figuring out what the heck was happening there :) [email protected] Review URL: https://codereview.chromium.org/1139673005
1 parent 286a2f3 commit 32f3c78

File tree

3 files changed

+19
-8
lines changed

3 files changed

+19
-8
lines changed

pkg/dev_compiler/lib/src/codegen/js_codegen.dart

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1172,7 +1172,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
11721172
return _getTemp(element, '${name.substring(1)}');
11731173
}
11741174

1175-
if (_isTemporary(element)) {
1175+
if (element is TemporaryVariableElement) {
11761176
if (name[0] == '#') {
11771177
return new JS.InterpolatedExpression(name.substring(1));
11781178
} else {
@@ -1775,7 +1775,7 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
17751775
// * create a new subtype of LocalVariableElementImpl to mark a temp.
17761776
var id =
17771777
new SimpleIdentifier(new StringToken(TokenType.IDENTIFIER, name, -1));
1778-
id.staticElement = new LocalVariableElementImpl.forNode(id);
1778+
id.staticElement = new TemporaryVariableElement.forNode(id);
17791779
id.staticType = type;
17801780
return id;
17811781
}
@@ -1802,8 +1802,6 @@ class JSCodegenVisitor extends GeneralizingAstVisitor with ConversionVisitor {
18021802
return value;
18031803
}
18041804

1805-
bool _isTemporary(Element node) => node.nameOffset == -1;
1806-
18071805
/// Returns a new expression, which can be be used safely *once* on the
18081806
/// left hand side, and *once* on the right side of an assignment.
18091807
/// For example: `expr1[expr2] += y` can be compiled as
@@ -2619,3 +2617,13 @@ bool _isJsPeerInterface(DartObjectImpl value) =>
26192617
// but we don't have summary support yet.
26202618
// bool _supportJsExtensionMethod(AnnotatedNode node) =>
26212619
// _getAnnotation(node, "SupportJsExtensionMethod") != null;
2620+
2621+
/// A special kind of element created by the compiler, signifying a temporary
2622+
/// variable. These objects use instance equality, and should be shared
2623+
/// everywhere in the tree where they are treated as the same variable.
2624+
class TemporaryVariableElement extends LocalVariableElementImpl {
2625+
TemporaryVariableElement.forNode(Identifier name) : super.forNode(name);
2626+
2627+
int get hashCode => identityHashCode(this);
2628+
bool operator ==(Object other) => identical(this, other);
2629+
}

pkg/dev_compiler/lib/src/codegen/js_names.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,10 @@ class _RenameVisitor extends VariableDeclarationVisitor {
8585

8686
declare(Identifier node) {
8787
var id = identifierKey(node);
88-
scope.declared.add(id);
88+
var notAlreadyDeclared = scope.declared.add(id);
89+
// Normal identifiers can be declared multiple times, because we don't
90+
// implement block scope yet. However temps should only be declared once.
91+
assert(notAlreadyDeclared || node is! TemporaryId);
8992
_markUsed(node, id, scope);
9093
}
9194

pkg/dev_compiler/test/codegen/expect/try_catch.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,10 +53,10 @@ var core = dart.import(core);
5353
function wub() {
5454
try {
5555
throw "on without exception parameter";
56-
} catch (e$) {
57-
if (dart.is(e$, core.String)) {
56+
} catch (e) {
57+
if (dart.is(e, core.String)) {
5858
} else
59-
throw e$;
59+
throw e;
6060
}
6161

6262
}

0 commit comments

Comments
 (0)