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

Issue 10425002: Resolve operators to int/double instead of num where possible (Closed)

Created:
8 years, 7 months ago by scheglov
Modified:
8 years, 7 months ago
Reviewers:
messick
CC:
reviews_dartlang.org
Visibility:
Public.

Description

Resolve operators to int/double instead of num where possible R=messick@google.com BUG= TEST= Committed: https://code.google.com/p/dart/source/detail?r=7933

Patch Set 1 #

Total comments: 2

Patch Set 2 : Test for ~/ with doubles #

Messages

Total messages: 3 (0 generated)
scheglov
8 years, 7 months ago (2012-05-23 18:22:34 UTC) #1
messick
LGTM after verifying ~/ https://chromiumcodereview.appspot.com/10425002/diff/1/compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right): https://chromiumcodereview.appspot.com/10425002/diff/1/compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java#newcode314 compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:314: case TRUNC: Have you verified ...
8 years, 7 months ago (2012-05-23 21:08:19 UTC) #2
scheglov
8 years, 7 months ago (2012-05-23 22:43:42 UTC) #3
https://chromiumcodereview.appspot.com/10425002/diff/1/compiler/java/com/goog...
File compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java (right):

https://chromiumcodereview.appspot.com/10425002/diff/1/compiler/java/com/goog...
compiler/java/com/google/dart/compiler/type/TypeAnalyzer.java:314: case TRUNC:
On 2012/05/23 21:08:19, messick wrote:
> Have you verified that both dart2js and the vm return an int in the case of
> <int> ~/ <int>? In any case, <double> ~/ <num> is specified to return <double>
> so you could add that.

1. Yes, I've checked that <int> ~/ <int> is <int> in VM and dart2js.

2. <double> ~/ <num> is already declared in type "double" as returning <double>,
so no need to tweaks here. But I've added test to ensure this.

Powered by Google App Engine
This is Rietveld 408576698