- Add support for binary operators

- Add support for binary operators
- Add support for ad-hoc hardcoded typing rules
- Add rules for int, double and num, for + and *

I'd like your opinion on the general strategy before adding dozens of ad-hoc
rules. So if you LGTM this CL, I'll add them in a later CL.

Note: this is different from trusting the types from the signatures of, say, int's methods because we can overload some methods. Defaulting to trusting the types might be a good replacement for what currently returns Dynamic.

Committed: https://code.google.com/p/dart/source/detail?r=14070

[sra1, 2012-10-12 05:00:51]

https://chromiumcodereview.appspot.com/11093078/diff/1/lib/compiler/implement...
File lib/compiler/implementation/types/concrete_types_inferrer.dart (right):

https://chromiumcodereview.appspot.com/11093078/diff/1/lib/compiler/implement...
lib/compiler/implementation/types/concrete_types_inferrer.dart:498: });
Since you are populating a cache, it might be more readable if you did that
directly.

void rule(op, recv, arg, result) { ...add to cache... }

var intClass = compiler.intClass;
...
rule(r'operator$add', intClass, intClass, intClass);
rule(r'operator$mul', intClass, intClass, intClass);
rule(r'operator$add', intClass, doubleClass, numClass);
etc.

Group it any way you please.

[polux, 2012-10-18 10:30:13]

I am now almost convinced this is not the right way to go eventually: I can
easily add rules of the form

{int} + {int} -> {int}

but not rules of the form:

{int} + !{int, double, num} -> {}

right now for "1 + 'a'" we infer dynamic instead of {}.

I suppose my only way out of this is not to populate the cache but to hook to
calls to the analysis of external functions and have a special logic there. 

Anyway, this is the fastest solution to get some benchmarks working in the short
term and we can change it later for something smarter.

https://chromiumcodereview.appspot.com/11093078/diff/1/lib/compiler/implement...
File lib/compiler/implementation/types/concrete_types_inferrer.dart (right):

https://chromiumcodereview.appspot.com/11093078/diff/1/lib/compiler/implement...
lib/compiler/implementation/types/concrete_types_inferrer.dart:498: });
On 2012/10/12 05:00:51, sra1 wrote:
> Since you are populating a cache, it might be more readable if you did that
> directly.
> 
> void rule(op, recv, arg, result) { ...add to cache... }
> 
> var intClass = compiler.intClass;
> ...
> rule(r'operator$add', intClass, intClass, intClass);
> rule(r'operator$mul', intClass, intClass, intClass);
> rule(r'operator$add', intClass, doubleClass, numClass);
> etc.
> 
> Group it any way you please.
> 
> 

Much better indeed. Done.

[karlklose, 2012-10-23 10:58:15]

LGTM with a few comments.

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
File lib/compiler/implementation/types/concrete_types_inferrer.dart (right):

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:434: return
product.iterator().next();
Can we construct this object more directly instead of using the product
iterator?

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:443: if
(receiverType.element.resolutionState == STATE_NOT_STARTED) {
use ensureResolved?

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:1290: final
Set<SourceString> operators = new Set<SourceString>()
When all operators are supported, we should be able to share this list with
other places that work on operators. Can you add a comment (for me)?

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:1323:
SourceString canonicalize(SourceString s) {
Perhaps 'canonicalMethodName'?

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:1351: if (method
!== null) {
Use != instead of !==.

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/cpa_...
File tests/compiler/dart2js/cpa_inference_test.dart (right):

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/cpa_...
tests/compiler/dart2js/cpa_inference_test.dart:375: testArith() {
testArith -> testArithmethicOperators.

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/cpa_...
tests/compiler/dart2js/cpa_inference_test.dart:387: a; b; c; d; e; f; g; h;
Can you add positive nesting tests, like (1 op 2) op 3 (and the same for num)?

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/mock...
File tests/compiler/dart2js/mock_compiler.dart (right):

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/mock...
tests/compiler/dart2js/mock_compiler.dart:58: abstract class num { operator
+(x); operator *(x); operator -(x); }
I am not sure we want to add these operators here. Perhaps we can use a local
mock core library in the inference test?

[polux, 2012-10-25 12:02:01]

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
File lib/compiler/implementation/types/concrete_types_inferrer.dart (right):

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:434: return
product.iterator().next();
Absolutely! Done.

On 2012/10/23 10:58:15, karlklose wrote:
> Can we construct this object more directly instead of using the product
> iterator?

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:443: if
(receiverType.element.resolutionState == STATE_NOT_STARTED) {
On 2012/10/23 10:58:15, karlklose wrote:
> use ensureResolved?

Done.

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:1290: final
Set<SourceString> operators = new Set<SourceString>()
On 2012/10/23 10:58:15, karlklose wrote:
> When all operators are supported, we should be able to share this list with
> other places that work on operators. Can you add a comment (for me)?

Done.

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:1323:
SourceString canonicalize(SourceString s) {
On 2012/10/23 10:58:15, karlklose wrote:
> Perhaps 'canonicalMethodName'?

Done.

http://codereview.chromium.org/11093078/diff/7001/lib/compiler/implementation...
lib/compiler/implementation/types/concrete_types_inferrer.dart:1351: if (method
!== null) {
On 2012/10/23 10:58:15, karlklose wrote:
> Use != instead of !==.

Done.

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/cpa_...
File tests/compiler/dart2js/cpa_inference_test.dart (right):

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/cpa_...
tests/compiler/dart2js/cpa_inference_test.dart:375: testArith() {
On 2012/10/23 10:58:15, karlklose wrote:
> testArith -> testArithmethicOperators.

Done.

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/cpa_...
tests/compiler/dart2js/cpa_inference_test.dart:387: a; b; c; d; e; f; g; h;
On 2012/10/23 10:58:15, karlklose wrote:
> Can you add positive nesting tests, like (1 op 2) op 3 (and the same for num)?

Done.

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/mock...
File tests/compiler/dart2js/mock_compiler.dart (right):

http://codereview.chromium.org/11093078/diff/7001/tests/compiler/dart2js/mock...
tests/compiler/dart2js/mock_compiler.dart:58: abstract class num { operator
+(x); operator *(x); operator -(x); }
On 2012/10/23 10:58:15, karlklose wrote:
> I am not sure we want to add these operators here. Perhaps we can use a local
> mock core library in the inference test?

Done.