lachrist / aran

JavaScript Code Instrumenter
https://lachrist.github.io/aran
MIT License
37 stars 4 forks source link

Passthrough cases in SwitchStatement #6

Closed turkja closed 5 years ago

turkja commented 7 years ago

Consider a following simple test case:

function testcase(n) {
    var r = 0;
    switch (n) {
        case 0x1:
        case 0x2:
        case 0x3:
            r = 3;
            break;
        case 0x4:
            r = 4;
            break;
    }
    return r;
}

When the switch case is instrumented, aran produces the following test cases for (1,2,3):

_meta_1 = _meta_2 === 1;
_meta_1 = _meta_2 === 2;
_meta_1 = _meta_2 === 3;

That means only the last test case is valid.

Maybe that should be written more like this:

(_meta_1 = _meta_2 === 1) || (_meta_1 = _meta_2 === 2) || (_meta_1 = _meta_2 === 3);

Quick experiment in visitors.SwitchStatement seems to do the trick:

  function fct2 (cse) {
    return "(" + str1 + "=" + (cse.test ? ctx.traps.test(
      ctx.traps.binary("===", str2, visit(ctx, ast, cse.test), ast.__min__),
      ast.__min__) : "true") + ")" + (cse.consequent.length ? ";" + cse.consequent.map(fct1).join("") : "||");
  }

But again, please check to be sure....

lachrist commented 7 years ago

Again, I do think that the transformation is correct. The condition variable being updated in chain is due to the empty cases. If you still think there is an error, could you provide me an example where the transformed code behave differently than the original code?

Cheers

_meta_.__global__ = _meta_.__global__ || (function() {
    return this
}());
_meta_.__eval__ = _meta_.__eval__ || eval;
_meta_.__apply__ = _meta_.__apply__ || (typeof Reflect === 'object' ? Reflect.apply : function(f, t, xs) {
    return f.apply(t, xs)
});
_meta_.__defineProperty__ = _meta_.__defineProperty__ || Object.defineProperty;
(testcase = function testcase(n) {
    var _meta_1, _meta_2, _meta_3;
    var r;
    (r = 0);;
    _meta_1 = false;
    _meta_2 = n;
    _meta_3: {
        _meta_1 = (_meta_2 === 0x1);_meta_1 = (_meta_2 === 0x2);_meta_1 = (_meta_2 === 0x3);
        if (_meta_1)(r = 3);
        if (_meta_1) break _meta_3;_meta_1 = (_meta_2 === 0x4);
        if (_meta_1)(r = 4);
        if (_meta_1) break _meta_3;
    }
    return r;
    return void 0
});
void 0;
var testcase;
turkja commented 7 years ago

Sure. If you run this code under node.js, it passes:

function testcase(n) {
    var r = 0;
    switch (n) {
        case 0x1:
        case 0x2:
            r = 1; break;
    }
    return r;
}
console.assert(testcase(1) == 1);

But when instrumented, the assert fails. I believe this happens because of this:

meta_1 = (_meta_2 === 0x1);_meta_1 = (_meta_2 === 0x2);

If _meta_2 == 1, _meta_1 is set to false anyways, because of the second statement.

This problem can also be verified with the demo page. With "4-Apply.js", if you put for example JSON.stringify(args) to options.log.

console.assert(testcase(1) == 1); returns false, even though it should return true.

lachrist commented 5 years ago

Works fine in aran@3.0.1

var Aran = require("aran");
const Acorn = require("Acorn");
var aran = Aran({format: "script"});
global[aran.namespace] = {};
global.eval(aran.setup());
global.eval(aran.weave(Acorn.parse(`
function testcase(n) {
    var r = 0;
    switch (n) {
        case 0x1:
        case 0x2:
            r = 1; break;
    }
    return r;
}
console.assert(testcase(1) == 1);
`),  []));