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

Issue 10532100: Issue 3532. Report error if one unit included twice (Closed)

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

Description

Patch Set 1 #

Total comments: 4

Patch Set 2 : Check uniqueness using URIs #

Total comments: 2
Unified diffs Side-by-side diffs Delta from patch set Stats (+40 lines, -3 lines) Patch
M compiler/java/com/google/dart/compiler/DartCompiler.java View 1 2 chunks +15 lines, -0 lines 0 comments Download
M compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java View 1 2 chunks +3 lines, -2 lines 2 comments Download
M compiler/javatests/com/google/dart/compiler/end2end/inc/IncrementalCompilation2Test.java View 1 1 chunk +22 lines, -0 lines 0 comments Download
M tests/co19/co19-compiler.status View 1 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 5 (0 generated)
scheglov
8 years, 6 months ago (2012-06-12 03:26:08 UTC) #1
Brian Wilkerson
https://chromiumcodereview.appspot.com/10532100/diff/1/compiler/java/com/google/dart/compiler/DartCompiler.java File compiler/java/com/google/dart/compiler/DartCompiler.java (right): https://chromiumcodereview.appspot.com/10532100/diff/1/compiler/java/com/google/dart/compiler/DartCompiler.java#newcode627 compiler/java/com/google/dart/compiler/DartCompiler.java:627: String path = sourceNode.getText(); I'm not sure this is ...
8 years, 6 months ago (2012-06-12 15:34:14 UTC) #2
scheglov
https://chromiumcodereview.appspot.com/10532100/diff/1/compiler/java/com/google/dart/compiler/DartCompiler.java File compiler/java/com/google/dart/compiler/DartCompiler.java (right): https://chromiumcodereview.appspot.com/10532100/diff/1/compiler/java/com/google/dart/compiler/DartCompiler.java#newcode627 compiler/java/com/google/dart/compiler/DartCompiler.java:627: String path = sourceNode.getText(); On 2012/06/12 15:34:14, Brian Wilkerson ...
8 years, 6 months ago (2012-06-12 18:17:51 UTC) #3
Brian Wilkerson
LGTM https://chromiumcodereview.appspot.com/10532100/diff/3001/compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java File compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java (right): https://chromiumcodereview.appspot.com/10532100/diff/3001/compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java#newcode22 compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java:22: UNIT_WAS_ALREADY_INCLUDED("Unit '%s' was already included"); nit: perhaps instead ...
8 years, 6 months ago (2012-06-12 18:26:29 UTC) #4
scheglov
8 years, 6 months ago (2012-06-12 18:38:20 UTC) #5
https://chromiumcodereview.appspot.com/10532100/diff/3001/compiler/java/com/g...
File compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java (right):

https://chromiumcodereview.appspot.com/10532100/diff/3001/compiler/java/com/g...
compiler/java/com/google/dart/compiler/DartCompilerErrorCode.java:22:
UNIT_WAS_ALREADY_INCLUDED("Unit '%s' was already included");
On 2012/06/12 18:26:29, Brian Wilkerson wrote:
> nit: perhaps instead of "included" we should say "sourced"?

"sourced" sounds as slang for me.
I've used to it however.

Spec says in "12.3 Includes":
An include directive species a URI where a Dart compilation unit that should
be incorporated into the current library may be found.

So, it looks for me that best word from from spec would be "incorporated" :-)
Or "included" by the name (but not keyword) or directive.
Well, this is mess...

Powered by Google App Engine
This is Rietveld 408576698