Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Array with double as size; motivations for an intermediate AST type-checker #26

Open
bys1 opened this issue May 25, 2023 · 3 comments
Open

Comments

@bys1
Copy link
Contributor

bys1 commented May 25, 2023

Got another error when trying to compile a class. This stuff is horrible to debug, but after messing around for a while I came up with a small example that can reproduce the error:

void compileTest() {
    loc l = |cwd:///src/main/rascal/Test.rsc|(314, 16, <18, 5>, <18, 21>);
    str  mcl = "io.usethesource.capsule.Map$Immutable";
    Class cl = class(
        object("Test"),
        modifiers = {\public(), \final()},
        methods = [
            main(
                "args",
                [
                    decl(double(), "i", init = dconst(0.)),
                    \while(
                        lt(
                            load("i"),
                            dconst(5.)
                        ),
                        [
                            decl(array(double()), "u", init = newArray(
                                array(double()),
                                dconst(3.)
                            ))
                        ]
                    ),
                    \return()
                ]
            )
        ],
        src=l
    );
    compileClass(cl, |cwd:///Test.class|);
}

The error does not occur when removing the while loop (moving the body of the while loop out of the loop). So the following program compiles without errors:

void compileTest() {
    loc l = |cwd:///src/main/rascal/Test.rsc|(314, 16, <18, 5>, <18, 21>);
    str  mcl = "io.usethesource.capsule.Map$Immutable";
    Class cl = class(
        object("Test"),
        modifiers = {\public(), \final()},
        methods = [
            main(
                "args",
                [
                    decl(array(double()), "u", init = newArray(
                        array(double()),
                        dconst(3.)
                    )),
                    \return()
                ]
            )
        ],
        src=l
    );
    compileClass(cl, |cwd:///Test.class|);
}

After my brain cells had recovered a bit from debugging, they discovered that this was an error on my side - Of course, using a double as array length (or index) is not possible, and an integer should be used. Using iconst instead worked.
What is weird, though, is that the decl in the loop gives the following error:

|jar+file:///Users/bys1/.m2/repository/org/rascalmpl/flybytes/0.2.0/flybytes-0.2.0.jar!/src/lang/flybytes/Compiler.rsc|(1637,272,<25,0>,<27,123>): Java("ArrayIndexOutOfBoundsException","")
	at compileClass(|jar+file:///Users/bys1/.m2/repository/org/rascalmpl/flybytes/0.2.0/flybytes-0.2.0.jar!/src/lang/flybytes/Compiler.rsc|(1902,5,<27,116>,<27,121>))
	at $shell$(|prompt:///|(0,14,<1,0>,<1,14>)ok

However, the decl without loop in the second code snippet compiles fine, although it is equally as wrong as the first one. Why is this the case? Maybe this is not really a bug, but it is certainly weird, because it made me think that the loop itself was problematic, while that was not the case at all.

Second question: Would it be possible in any way to give more descriptive errors in situations like this? Something like "type double cannot be used for array size/index" or even a more general error like "integer expected, double given" would be much more useful than a Java ArrayIndexOutOfBoundsException with no Java stack trace.

@jurgenvinju
Copy link
Member

Thanks @bys1 ; this is worth a lot to me.

The ASM library that we use to assemble the bytecode is notorious in not providing error messages at all. This is to optimize for speed.

Flybytes surfs a little bit differently. We try to provide error messages if we can do this in O(1), so constant time overhead wrt to size of the program.

So any small and fast check we can do, or diagnosis after an exception, let's do it.

Finally, some JVM code just works even though it violates Java's type system. I'd like to keep it that way such that more code can be generated for DSLs than for Java. In particular some methods can be called even though the static type of an object does not align, but also some instructions implicitly coerce datatypes, saving precious stack operations. Here we have to tred carefully, for example by collecting warnings rather than bailing out.

@jurgenvinju
Copy link
Member

What is weird, though, is that the decl in the loop gives the following error

This is caused by doubles taking up more stack slots than integers do.

@jurgenvinju
Copy link
Member

jurgenvinju commented May 26, 2023

I've been thinking about two alternative solutions:

  • introduce inline type-checking while emitting code in Java
  • introduce an external type-checker implemented in Rascal

The first is problematic for several reasons:

  • It will slow down code generators
  • When a compiler is finished, the input code is type-checked, the code generator is correct, and so any additional checking is wasteful
  • The compiler written in Java is "complex enough" as it is.

The second solution appeals to me:

  • can be used for a priori and a posteriori checking:
    • use it as a quality gate during the design of a code generator before the call to the flybytes compiler. This produces better error messages and avoids the long debugging sessions
    • use it as a diagnostics tool; when the flybytes compiler or the JVM class loader fails, rollback to the intermediate AST and type-check it to get an explanation. This will become a tool when your code generator is almost finished.
    • use it as an error messaging tool for your DSL. When we do detect a problem during code generation after deployment, at least this could give a bit more insight to the user than "ArrayIndexOutOfBounds", and also it will help diagnostics if they report an issue with your DSL.
  • With this solution we pay only when we require the feature and not when we do not.
  • Written in pure Rascal.

@jurgenvinju jurgenvinju changed the title Array with double as size Array with double as size; motivations for an intermediate AST type-checker May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants