b9org / b9

An educational JS virtual machine based on Eclipse OMR
http://www.base9.xyz
Apache License 2.0
45 stars 24 forks source link

Implement for loops in the frontend #118

Closed rwy7 closed 6 years ago

rwy7 commented 6 years ago

There were some other issues with compiling JMP in the jit, and compiling update-assignment expressions in the JS FE, which I fixed. I also added two small for-loop tests.

coveralls commented 6 years ago

Pull Request Test Coverage Report for Build 415


Totals Coverage Status
Change from base Build 414: -0.009%
Covered Lines: 2182
Relevant Lines: 2925

💛 - Coveralls
youngar commented 6 years ago

Another broken call to handle().

    // Iterate expressions array
    this.handleSequenceExpression = function (func, sequence) {
        var expressions = sequence.expressions;
        var droplast = !decl.isParameter;

        for (expression in sequence.expressions.slice(0, -1)) {
            this.handle(func, expression);
            func.instructions.push(new Instruction("DROP"));
        }

        var last = sequence.expressions.slice(-1)[0];
        this.handle(last);

        if (!sequence.isParameter) {
            func.instructions.push(new Instruction("DROP"));
        }
    };
rwy7 commented 6 years ago

I'll open an issue for the second busted handle and deal with it later, cool?

youngar commented 6 years ago

@rwy0717 Sounds good, we can fix that later.

Unrelated question: do we want the coveralls check failing if we reduce test coverage?

rwy7 commented 6 years ago

I was thinking about the same thing. Really, it's up to us to decide how to interpret the results. The threshold is configurable. I thought it was a little silly that it complains about a 0.009 % reduction in coverage, so I upped the threshold to 1%. I also disabled the comment--which I'm open to reenabling.