AzureMarker / intellij-lalrpop

Jetbrains plugin for the LALRPOP parser-generator
MIT License
16 stars 2 forks source link

Bug from #26 implementation of `LpAction.resolveType` #37

Closed dnbln closed 3 years ago

dnbln commented 3 years ago

Missed a bug here: https://github.com/Mcat12/intellij-lalrpop/pull/26/files#diff-d4de5725837b447f72910db82168d2607db0b58a7af2e801cc0874e5e47f5a2eR120

(Because I always name my macro parameters Rule I didn't catch it yet). Just now when I needed 2 parameters and playing around with the type inlays it became apparent: the substitution will always yield unknowns if the macro parameters at the beginning of the resolveType chain aren't given their own unit structs here.

AzureMarker commented 3 years ago

Can you explain the bug a bit more? It sounds like sometimes we don't fill in all of the type variables (via LpMacroArguments), causing some type variables to be unbound in the resolved type?

dnbln commented 3 years ago

Well let's take an example:

grammar;
Macro1<A> = A => <>;
Macro2<B> = Macro1<B>;

Here, while resolving the type of Macro1, the plugin just goes directly to trying to infer the type of the function. Here's what it would look like:

mod __intellij_lalrpop {
    //imports
    struct A;
    fn __intellij_lalrpop<A>(__intellij_lalrpop_noname_0: A) {
        __intellij_lalrpop_noname_0
    }
}

It's correctly able to tell the return type of the function is A and then just provides the substitution A=>A, which makes sense because A is also defined as an unit struct.

Entering Macro2 happens with:

LpMacroArguments(listOf(LpMacroArgument(name="B", rustType="B")))

Then, going from Macro2 to Macro1:

LpMacroArguments(listOf(LpMacroArgument(name="A", rustType="B")))

Now, the whole module above is the same, and the unit structs don't change because they are only taken from the nonterminal around the action code and the grammar_decl.

mod __intellij_lalrpop {
    //imports
    struct A;
    fn __intellij_lalrpop<A>(__intellij_lalrpop_noname_0: A) {
        __intellij_lalrpop_noname_0
    }
}

But this time around, the substitution is A=>B rather than A=>A, so the rust plugin is now looking for B, which it cannot find.

The fix suggested in #38 replaces struct A; with struct B;, by creating unit structs for the macro parameters of Macro2 instead of Macro1.