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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 208423012: Add helpers library to dart2js. (Closed)

Created:
6 years, 9 months ago by Johnni Winther
Modified:
5 years, 10 months ago
Reviewers:
karlklose
CC:
reviews_dartlang.org, ahe
Visibility:
Public.

Description

Add helpers library to dart2js. BUG= R=karlklose@google.com Committed: https://code.google.com/p/dart/source/detail?r=34410

Patch Set 1 #

Total comments: 8
Unified diffs Side-by-side diffs Delta from patch set Stats (+233 lines, -615 lines) Patch
M sdk/lib/_internal/compiler/implementation/compile_time_constants.dart View 1 chunk +0 lines, -20 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/compiler.dart View 5 chunks +19 lines, -11 lines 4 comments Download
M sdk/lib/_internal/compiler/implementation/dart2js.dart View 1 chunk +1 line, -1 line 0 comments Download
M sdk/lib/_internal/compiler/implementation/dart2jslib.dart View 2 chunks +2 lines, -0 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/elements/elements.dart View 3 chunks +2 lines, -3 lines 0 comments Download
A sdk/lib/_internal/compiler/implementation/helpers/debug_collection.dart View 1 chunk +67 lines, -0 lines 2 comments Download
A + sdk/lib/_internal/compiler/implementation/helpers/expensive_map.dart View 1 chunk +1 line, -1 line 0 comments Download
A + sdk/lib/_internal/compiler/implementation/helpers/expensive_set.dart View 1 chunk +1 line, -1 line 0 comments Download
A sdk/lib/_internal/compiler/implementation/helpers/helpers.dart View 1 chunk +26 lines, -0 lines 0 comments Download
A + sdk/lib/_internal/compiler/implementation/helpers/trace.dart View 3 chunks +13 lines, -134 lines 2 comments Download
M sdk/lib/_internal/compiler/implementation/js_backend/backend.dart View 3 chunks +9 lines, -53 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/js_emitter/declarations.dart View 1 chunk +0 lines, -3 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/source_map_builder.dart View 2 chunks +0 lines, -10 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/use_unused_api.dart View 6 chunks +33 lines, -2 lines 0 comments Download
D sdk/lib/_internal/compiler/implementation/util/expensive_map.dart View 1 chunk +0 lines, -73 lines 0 comments Download
D sdk/lib/_internal/compiler/implementation/util/expensive_set.dart View 1 chunk +0 lines, -124 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/util/util.dart View 2 chunks +0 lines, -173 lines 0 comments Download
M sdk/lib/_internal/compiler/implementation/warnings.dart View 1 chunk +11 lines, -1 line 0 comments Download
M tests/compiler/dart2js/analyze_helper.dart View 3 chunks +12 lines, -3 lines 0 comments Download
M tests/compiler/dart2js/analyze_unused_dart2js_test.dart View 1 chunk +36 lines, -2 lines 0 comments Download

Messages

Total messages: 8 (0 generated)
Johnni Winther
6 years, 9 months ago (2014-03-25 13:48:01 UTC) #1
karlklose
LGTM. https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/helpers/debug_collection.dart File sdk/lib/_internal/compiler/implementation/helpers/debug_collection.dart (right): https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/helpers/debug_collection.dart#newcode42 sdk/lib/_internal/compiler/implementation/helpers/debug_collection.dart:42: V v = ifAbsent(); Indentation is off. https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/helpers/trace.dart ...
6 years, 9 months ago (2014-03-25 14:22:18 UTC) #2
Johnni Winther
Committed patchset #1 manually as r34410 (presubmit successful).
6 years, 9 months ago (2014-03-26 07:32:50 UTC) #3
Johnni Winther
https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/helpers/debug_collection.dart File sdk/lib/_internal/compiler/implementation/helpers/debug_collection.dart (right): https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/helpers/debug_collection.dart#newcode42 sdk/lib/_internal/compiler/implementation/helpers/debug_collection.dart:42: V v = ifAbsent(); On 2014/03/25 14:22:18, karlklose wrote: ...
6 years, 9 months ago (2014-03-26 07:35:04 UTC) #4
ahe
https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/compiler.dart File sdk/lib/_internal/compiler/implementation/compiler.dart (left): https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/compiler.dart#oldcode1376 sdk/lib/_internal/compiler/implementation/compiler.dart:1376: void reportHere(Spannable node, String debugMessage) { Is there an ...
6 years, 8 months ago (2014-04-10 11:06:32 UTC) #5
Johnni Winther
https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/compiler.dart File sdk/lib/_internal/compiler/implementation/compiler.dart (left): https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/compiler.dart#oldcode1376 sdk/lib/_internal/compiler/implementation/compiler.dart:1376: void reportHere(Spannable node, String debugMessage) { On 2014/04/10 11:06:32, ...
6 years, 8 months ago (2014-04-10 11:11:45 UTC) #6
ahe
https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/compiler.dart File sdk/lib/_internal/compiler/implementation/compiler.dart (left): https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/implementation/compiler.dart#oldcode1376 sdk/lib/_internal/compiler/implementation/compiler.dart:1376: void reportHere(Spannable node, String debugMessage) { On 2014/04/10 11:11:45, ...
6 years, 8 months ago (2014-04-10 11:43:46 UTC) #7
Johnni Winther
6 years, 8 months ago (2014-04-10 12:04:21 UTC) #8
Message was sent while issue was closed. 
https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/i...
File sdk/lib/_internal/compiler/implementation/compiler.dart (left):

https://codereview.chromium.org/208423012/diff/1/sdk/lib/_internal/compiler/i...
sdk/lib/_internal/compiler/implementation/compiler.dart:1376: void
reportHere(Spannable node, String debugMessage) {
On 2014/04/10 11:43:46, ahe wrote:
> On 2014/04/10 11:11:45, Johnni Winther wrote:
> > On 2014/04/10 11:06:32, ahe wrote:
> > > Is there an alternative to this?
> > 
> > It is moved to helpers/helpers.dart to ensure that production code does not
> use
> > it (checked by analyze_used_dart2js_test).
> 
> That means I have to import helpers/helpers.dart. That makes it virtually
> useless, as the method only serves a purpose when it is *easier* to use than
> writing the code below inline. I think there are better ways to ensure that
> "reportHere" is never called. For example, by running analyze-only on
> dart2js.dart, and checking that the method wasn't enqueued.
> 
> I would argue that I'm now more likely to accidentally include
> MessageKind.GENERIC in production code, because I'm not going to import a
helper
> library.

Why is it a problem to import 'helpers/helpers.dart'?

If MessageKind.GENERIC shouldn't be used in production code (which it is) we
should enforce and check it.

Powered by Google App Engine
This is Rietveld 408576698