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

Issues Search
    My Issues | Starred     Open | Closed | All

Issue 1635053002: dart2js: Match the runtime int is check semantics at compile time. (Closed)

Created:
4 years, 11 months ago by Kevin Millikin (Google)
Modified:
4 years, 11 months ago
Reviewers:
asgerf
CC:
reviews_dartlang.org
Base URL:
git@github.com:dart-lang/sdk.git@master
Target Ref:
refs/heads/master
Visibility:
Public.

Description

dart2js: Match the runtime int is check semantics at compile time. BUG= R=asgerf@google.com Committed: https://github.com/dart-lang/sdk/commit/ba884f7f9fdf464eab3b46f8b2e609f22c82d171

Patch Set 1 #

Total comments: 1

Patch Set 2 : Implement the same semantics at compile time and run time. #

Total comments: 1

Patch Set 3 : Incorporate review comments. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+19 lines, -6 lines) Patch
M pkg/compiler/lib/src/constants/values.dart View 1 2 chunks +6 lines, -0 lines 0 comments Download
M pkg/compiler/lib/src/js_backend/constant_system_javascript.dart View 1 2 1 chunk +11 lines, -2 lines 0 comments Download
M tests/co19/co19-dart2js.status View 1 2 chunks +2 lines, -4 lines 0 comments Download

Messages

Total messages: 8 (3 generated)
Kevin Millikin (Google)
https://codereview.chromium.org/1635053002/diff/1/tests/co19/co19-dart2js.status File tests/co19/co19-dart2js.status (left): https://codereview.chromium.org/1635053002/diff/1/tests/co19/co19-dart2js.status#oldcode9633 tests/co19/co19-dart2js.status:9633: -LibTest/core/double/INFINITY_A01_t04: Pass # Please triage this failure. Pass is ...
4 years, 11 months ago (2016-01-26 13:04:12 UTC) #2
asgerf
lgtm
4 years, 11 months ago (2016-01-26 13:19:23 UTC) #3
Kevin Millikin (Google)
As discussed offline, the 'right' thing to do here is to implement the same semantics ...
4 years, 11 months ago (2016-01-26 14:03:32 UTC) #4
asgerf
LGTM with doc comment https://codereview.chromium.org/1635053002/diff/20001/pkg/compiler/lib/src/js_backend/constant_system_javascript.dart File pkg/compiler/lib/src/js_backend/constant_system_javascript.dart (right): https://codereview.chromium.org/1635053002/diff/20001/pkg/compiler/lib/src/js_backend/constant_system_javascript.dart#newcode277 pkg/compiler/lib/src/js_backend/constant_system_javascript.dart:277: // Integer checks don't verify ...
4 years, 11 months ago (2016-01-26 14:59:18 UTC) #6
Kevin Millikin (Google)
4 years, 11 months ago (2016-01-26 15:11:06 UTC) #8
Message was sent while issue was closed. 
Committed patchset #3 (id:40001) manually as
ba884f7f9fdf464eab3b46f8b2e609f22c82d171 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698