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

Issue 9600049: Use NodeList where possible. (Closed)

Created:
8 years, 9 months ago by scheglov
Modified:
8 years, 9 months ago
Reviewers:
Brian Wilkerson
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Use NodeList where possible. R=brianwilkerson@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=5036

Patch Set 1 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+252 lines, -193 lines) Patch
M compiler/java/com/google/dart/compiler/ast/DartArrayLiteral.java View 2 chunks +3 lines, -3 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartBlock.java View 2 chunks +5 lines, -5 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartClass.java View 3 chunks +9 lines, -9 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartFieldDefinition.java View 2 chunks +3 lines, -3 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartFunction.java View 2 chunks +6 lines, -10 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartFunctionTypeAlias.java View 2 chunks +6 lines, -6 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartInvocation.java View 1 chunk +2 lines, -2 lines 2 comments Download
M compiler/java/com/google/dart/compiler/ast/DartMapLiteral.java View 2 chunks +3 lines, -3 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartMethodDefinition.java View 2 chunks +3 lines, -3 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartParameter.java View 3 chunks +10 lines, -3 lines 2 comments Download
M compiler/java/com/google/dart/compiler/ast/DartParameterizedTypeNode.java View 2 chunks +3 lines, -11 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartStringInterpolation.java View 2 chunks +6 lines, -6 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartSwitchMember.java View 2 chunks +3 lines, -7 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartSwitchStatement.java View 2 chunks +3 lines, -3 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartToSourceVisitor.java View 1 chunk +1 line, -1 line 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartTryStatement.java View 2 chunks +3 lines, -3 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartTypeNode.java View 3 chunks +5 lines, -5 lines 2 comments Download
M compiler/java/com/google/dart/compiler/ast/DartTypedLiteral.java View 2 chunks +3 lines, -16 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartUnit.java View 5 chunks +8 lines, -44 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/DartVariableStatement.java View 3 chunks +3 lines, -3 lines 0 comments Download
M compiler/java/com/google/dart/compiler/ast/LibraryUnit.java View 3 chunks +9 lines, -8 lines 0 comments Download
A compiler/java/com/google/dart/compiler/ast/NodeList.java View 1 chunk +111 lines, -0 lines 0 comments Download
M compiler/java/com/google/dart/compiler/parser/CommentPreservingParser.java View 1 chunk +1 line, -1 line 0 comments Download
M compiler/java/com/google/dart/compiler/parser/DartParser.java View 2 chunks +6 lines, -6 lines 0 comments Download
M compiler/java/com/google/dart/compiler/resolver/CompileTimeConstantAnalyzer.java View 1 chunk +1 line, -1 line 0 comments Download
M compiler/java/com/google/dart/compiler/resolver/CompileTimeConstantResolver.java View 1 chunk +1 line, -1 line 0 comments Download
M compiler/java/com/google/dart/compiler/resolver/ResolveVisitor.java View 1 chunk +1 line, -1 line 0 comments Download
M compiler/java/com/google/dart/compiler/resolver/Resolver.java View 1 chunk +1 line, -1 line 0 comments Download
M compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java View 1 chunk +1 line, -1 line 0 comments Download
M compiler/javatests/com/google/dart/compiler/resolver/NegativeResolverTest.java View 1 chunk +10 lines, -10 lines 0 comments Download
M compiler/javatests/com/google/dart/compiler/resolver/ResolverTestCase.java View 1 chunk +1 line, -1 line 0 comments Download
M compiler/javatests/com/google/dart/compiler/type/TypeAnalyzerCompilerTest.java View 1 chunk +1 line, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/dom/PropertyGetter.java View 1 chunk +1 line, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/dom/PropertyLocator.java View 1 chunk +1 line, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/completion/CompletionEngine.java View 1 chunk +1 line, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/completion/ScopedNameFinder.java View 1 chunk +1 line, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/completion/ast/FunctionCompleter.java View 1 chunk +1 line, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/formatter/CodeFormatterVisitor.java View 3 chunks +3 lines, -3 lines 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core/src/com/google/dart/tools/core/internal/model/CompilationUnitImpl.java View 1 chunk +1 line, -1 line 0 comments Download
M editor/tools/plugins/com.google.dart.tools.core_test/src/com/google/dart/tools/core/internal/model/DartModelManagerTest.java View 5 chunks +11 lines, -6 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
scheglov
8 years, 9 months ago (2012-03-06 16:06:53 UTC) #1
Brian Wilkerson
LGTM https://chromiumcodereview.appspot.com/9600049/diff/1/compiler/java/com/google/dart/compiler/ast/DartInvocation.java File compiler/java/com/google/dart/compiler/ast/DartInvocation.java (right): https://chromiumcodereview.appspot.com/9600049/diff/1/compiler/java/com/google/dart/compiler/ast/DartInvocation.java#newcode37 compiler/java/com/google/dart/compiler/ast/DartInvocation.java:37: private final NodeList<DartExpression> args = NodeList.create(this); Let's rename ...
8 years, 9 months ago (2012-03-06 16:39:44 UTC) #2
scheglov
8 years, 9 months ago (2012-03-06 17:11:45 UTC) #3
https://chromiumcodereview.appspot.com/9600049/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/ast/DartInvocation.java (right):

https://chromiumcodereview.appspot.com/9600049/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/ast/DartInvocation.java:37: private final
NodeList<DartExpression> args = NodeList.create(this);
On 2012/03/06 16:39:45, Brian Wilkerson wrote:
> Let's rename this to "arguments" (in a later CL is fine).

Done.

https://chromiumcodereview.appspot.com/9600049/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/ast/DartParameter.java (right):

https://chromiumcodereview.appspot.com/9600049/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/ast/DartParameter.java:21: private final
NodeList<DartParameter> functionParameters;
On 2012/03/06 16:39:45, Brian Wilkerson wrote:
> Why isn't this always getting initialized like we do in other places?

I have to do this to distinguish between "normal" parameter and "function"
parameter.

https://chromiumcodereview.appspot.com/9600049/diff/1/compiler/java/com/googl...
File compiler/java/com/google/dart/compiler/ast/DartTypeNode.java (right):

https://chromiumcodereview.appspot.com/9600049/diff/1/compiler/java/com/googl...
compiler/java/com/google/dart/compiler/ast/DartTypeNode.java:22:
this(identifier, ImmutableList.<DartTypeNode>of());
On 2012/03/06 16:39:45, Brian Wilkerson wrote:
> Given that NodeList.addAll handles an argument value of null, I don't think we
> need to create a list here; I think we can just pass in null.

Done.
Thank you for catching this.

Powered by Google App Engine
This is Rietveld 408576698