Right now the range analysis code is only tested indirectly, through jit-tests, jstests, etc. But this problem is narrowly scoped enough that we can, and should, test the range analysis code directly at the C++ level, in jsapi-tests (that use purely internal headers, but it's okay for JSAPI tests to do that). It's a whole lot easier constructing debugging a jsapi-test of range analysis stuff, than it is of JS code that happens to invoke the range analysis code in the same way. (Also, JS might constant-fold and do other things such that it's difficult to impossible to invoke range analysis with the precise desired inputs, which might hide bugs thought impossible.) There's a little messiness here in that Range is TempObject right now, but I imagine throwing some C++ templates at the problem, and having (say) |template<class Base> class RangeImpl : public Base| with |typedef RangeImpl<TempObject> Range;| or something would solve things pretty nicely.

js/src/jsapi-tests/testJitRValueAlloc.cpp is now an example of a JIT test in jsapi-tests.

This bug is not a good first bug, as it requires a bit of knowledge about Range Analysis. I would recommend to have a look at the work being done in order to improve our documentation on Range Analysis (Bug 1028274) The goal of adding unit-test for Range Analysis is first to test all functions which are made on Range [1], and ensure that if we provide any value within the input range, then the result is within the output range. Among interesting values which are interesting in terms of boundaries of inputs / outputs ranges, we have: ±Inf ±Math.pow(2,52) (± 1) ±Math.pow(2,32) (± 1) ±Math.pow(2,31) (± 1) ±Math.pow(2,n) (± 0.5) [n < 32] ±0 So there is a lot of potential test cases to make and a lot of values to iterate over. [1] http://dxr.mozilla.org/mozilla-central/source/js/src/jit/RangeAnalysis.h?from=RangeAnalysis.h#398-418

Bug 1036037 added js/src/jsapi-tests/testJitFoldsTo.cpp, which is an example of a JIT test in jsapi-tests that builds a MIRGraph.

This test redoes the test from bug 1094052 to be more unit-test-like, calling the Range class functions directly, rather than going through all the trouble of building a MIRGraph. I think this is a good example of what we want unit tests for the Range class to look like. It also adds a test for bug 1093356, which is a good example of a testcase which does need to build a full MIRGraph.

Comment on attachment 8519105 [details] [diff] [review] range-unit-test.patch Review of attachment 8519105 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsapi-tests/testJitRangeAnalysis.cpp @@ +52,5 @@ > + Range *zero = Range::NewDoubleSingletonRange(func.alloc, 0.0); > + Range *half = Range::NewDoubleSingletonRange(func.alloc, 0.5); > + Range *one = Range::NewDoubleSingletonRange(func.alloc, 1.0); > + Range *threehalves = Range::NewDoubleSingletonRange(func.alloc, 1.5); > + Range *inf = Range::NewDoubleSingletonRange(func.alloc, mozilla::PositiveInfinity<double>()); nit: I think that using variables with identical length would improve the readability of this test: _nan ninf n1_5 n1_0 n0_5 n0_0 p0_0 p0_5 p1_0 p1_5 pinf

Done. https://hg.mozilla.org/integration/mozilla-inbound/rev/b27100f83983