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

Add meta variables to SDF3 Stratego mix syntax test #65

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

Apanatshka
Copy link
Contributor

This adds explicit SDF3 rules that introduce some meta-variables. These used to be introduced in a variables section in SDF2, which was a little magical. SDF3 normaliser seems to throw away a variables section in SDF3 files. The limitation of explicit context-free rules is that this doesn't work for meta-list-variables. So perhaps we should fix the support for the variables section, or for explicit list variable rules. You can write them, like:

Stmt+.meta-list-var = StmtListVar

But the parse result isn't quite right.

@Apanatshka
Copy link
Contributor Author

I commented some Stratego code that's not parsed quite right. That's:

  reverse-stmts-concrete:
    |[ stmt1* ]| ->
    |[ stmt2* ]|
  where stmt2* := <reverse> stmt1*

It's currently parsed as:

      , RDefNoArgs(
          "reverse-stmts-concrete"
        , Rule(
            NoAnnoList(ToTerm(Program(["stmt1*"])))
          , NoAnnoList(ToTerm(Program(["stmt2*"])))
          , [ WhereClause(
                Assign(
                  Var(ListVar("stmt2*"))
                , App(CallNoArgs(SVar("reverse")), Var(ListVar("stmt1*")))
                )
              )
            ]
          )
        )

It should instead be:

      , RDefNoArgs(
          "reverse-stmts-concrete"
        , Rule(
            NoAnnoList(ToTerm(Program(meta-list-var("stmt1*"))))
          , NoAnnoList(ToTerm(Program(meta-list-var("stmt2*"))))
          , [ WhereClause(
                Assign(
                  Var(ListVar("stmt2*"))
                , App(CallNoArgs(SVar("reverse")), Var(ListVar("stmt1*")))
                )
              )
            ]
          )
        )

The AST gets more interesting when you use meta list variables as a sublist. It'll be a list that uses Conc constructors to combine the meta list variable with the normal Stratego list parts.

I think support for this list stuff mostly needs to be added to the imploder.

@jasperdenkers
Copy link
Contributor

The imploder recognizes lists meta-variables by the constructor meta-listvar instead of meta-list-var. Fixed in your example in 0625814.

With this fix, a simple test for the "reverse statements" strategy that you set up now works (ed74418).

The AST gets more interesting when you use meta list variables as a sublist. It'll be a list that uses Conc constructors to combine the meta list variable with the normal Stratego list parts.

I'm not sure how to reproduce/verify this. Could you check whether this case is also handled correctly when using the meta-listvar constructor?

@Apanatshka
Copy link
Contributor Author

Oh, I got the constructor name wrong then? Sorry about that. I'll add a test for the meta list variables in the middle of a list.

@Apanatshka
Copy link
Contributor Author

Looks like Conc is not well supported, though that might be a Stratego issue more than an SDF issue. I'll look into that. But in the mean time, notice how the test I added doesn't actually surround the meta-listvar. If you try that the parser fails. I suspect that's because lists are these days defined as Statement+ = Statement+ Statement instead of Statement+ = Statement+ Statement+ {left}.

@jasperdenkers
Copy link
Contributor

I think you're right it is because of the Statement+ = Statement+ Statement normalization.

Adding the following does help:

Stmt+.Snoc = Stmt+ StmtListVar

However, somehow, the test fails:

Expected: Program([Stmt(Var("a")),Stmt(Var("x")),Stmt(Var("y")),Stmt(Var("a"))])
Got: Program([Stmt(Var("x")),Stmt(Var("y"))])

See 47937bc. Is that because of something on Stratego's side?

@Apanatshka
Copy link
Contributor Author

Apanatshka commented Mar 8, 2023

TODO

  • Add hack to Stratego 1 and 2 compilers in meta-explode to handle unexpected non-list non-meta-(list)var in Conc constructor.
  • Fix the Stratego 1 custom imploder in the compiler to handle create Conc constructors with single children in a list (i.e. bugfix the Cons/Snoc cases). Has to be done here instead of in meta-explode, because by then you don't know if a FromTerm constructor was because of an element in the list (e.g. a, ~elem, b) or because of a sublist (e.g. a, ~*elems, b).
  • Add more tests to see if this grammar leads to strange or ambiguous parses that make the compiler blow up or give wrong code.
  • Test with JSGLR2 (through Stratego 2?) that the parse result is the same / this approach to mix syntax keeps working.

@Apanatshka
Copy link
Contributor Author

Apanatshka commented Mar 9, 2023

@jasperdenkers I've added a Stratego 2 version of the integrationtest, and it looks like the JSGLR2 imploder does not create Conc lists, instead it puts the list meta variables and such in as an element of the list.

Apanatshka and others added 3 commits March 9, 2023 16:59
# Conflicts:
#	org.metaborg.sdf3.meta.integrationtest/editor/Main.esv
"Update mix syntax integrationtest to import module that defined Module start symbol"
@jasperdenkers
Copy link
Contributor

@Apanatshka after merging master and applying the same fix as you did in master (ab146a6) to the Stratego 2 version of the integration test (fc6a669), building the Stratego 2 version of the integration test fails with the following:

[ERROR] ERROR in file:///Users/jasper/spoofax/releng/sdf/sdf3.stratego2.integrationtest/trans/swap.str2:38
[ERROR]         stmt |[ ~exp:e1 + ~exp:e2; ]| ->
[ERROR]              ^
[ERROR] Unexpected input

Since the rest of the setup is similar, is this because the Stratego 2 grammar is different and thus the mix syntax should be set up differently? Did you not experience this failure?

@Apanatshka
Copy link
Contributor Author

I've taken some time off (this week and next), but before then I quickly changed the Stratego 2 grammar to have all unique constructor names so we can set up mix syntax in Spoofax 3. I didn't see any integration test errors on master, but I didn't check this branch. Given how the tests are set up without any assumption about Stratego 2 abstract syntax constructor names, I'm surprised this is now failing.

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

Successfully merging this pull request may close these issues.

2 participants