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

Scala 2 libray TASTy compatibility #108

Closed
nicolasstucki opened this issue Mar 14, 2024 · 7 comments
Closed

Scala 2 libray TASTy compatibility #108

nicolasstucki opened this issue Mar 14, 2024 · 7 comments

Comments

@nicolasstucki
Copy link

We found that this library does not work with the current Scala 2 library TASTy

Please help us minimize this issue to see if it is something expected, a bug in this code base, or a bug in the compiler/library.

@davesmith00000
Copy link
Member

Digging into this a bit, the error messages is:

Exception occurred while executing macro expansion.
ultraviolet.datatypes.ShaderError$UBORead: [ultraviolet] A UBO field was misread.

Coming from here:

else throw ShaderError.UBORead("A UBO field was misread.")

What's happening (don't know why yet) is that it tries to summon an equal number of "precision", label, and type name entries. The type names and the precisions come back with 2 values, not doubt corresponding to the two fields (see below), but the labels are missing (empty list). This is the test case class it's trying to read:

case class UBO1(TIME: highp[Float], val VIEWPORT_SIZE: vec2)

I've grabbed a snapshot of the syntax tree, and I'll see if I can work out where it's going wrong...

@davesmith00000
Copy link
Member

Ok, so what's happened is that the expected structure has changed. I can make it compile by adding one new case clause. The tests fail after that, but that's another problem, screenshot below for anyone interested:

image

The request was to minimise the issue, @nicolasstucki. 🤔

Well, the expected structure was this:

case Inlined(
              _,
              _,
              Typed(
                Block(
                  List(
                    ValDef(
                      _,
                      _,
                      Some(
                        TypeApply(
                          Select(
                            Select(
                              Apply(
                                _,
                                List(Literal(StringConstant(label)))
                              ),
                              _
                            ),
                            _
                          ),
                          _
                        )
                      )
                    )
                  ),
                  _
                ),
                _
              )
            ) =>

...and is now this:

case Inlined(
              _,
              _,
              Typed(
                Block(
                  List(
                    ValDef(
                      _,
                      _,
                      Some(TypeApply(Select(Literal(StringConstant(label)), _), _))
                    )
                  ),
                  _
                ),
                _
              )
            ) =>

However, I expect this is a me problem, and what I should be doing is less fragile matching. Pro-tips welcome, but I'm thinking I just need to walk the tree until I find the Literal(StringConstant that I'm after and discard everything else.

@nicolasstucki
Copy link
Author

The request was to minimise the issue, @nicolasstucki. 🤔

Minimize, find differences, find issues.... Anything that can helps us make sure this will work with your project when we eventually stabilize it.

@nicolasstucki
Copy link
Author

The difference in the pattern is in

- Some(TypeApply(Select(Select(Apply(_/*f*/, List(Literal(StringConstant(label)))), _/*x*/), /*g*/), _/*T1,..Tn*/))
+ Some(TypeApply(Select(Literal(StringConstant(label)), _/*h*/), _/*U1,..Un*/))

which makes seems to indicate that we are matching on an expression of the form

- f("label").x.g[T1,..,Tn]
+ "label".h[U1,..,Un]

@davesmith00000 what is the actual expression you are matching on? The tree.show and tree.show(using Printer.TreeStructure).

I assume that it is something like

```diff
- f("label").x.asInstanceOf[T]
+ "label".asInstanceOf[T]

@nicolasstucki
Copy link
Author

Do you know statically which f, x you want to match on?

@davesmith00000
Copy link
Member

Thanks @nicolasstucki, I'm not immediately following all of that (sorry, my macro-fu is weak 😅), but I'll try and work through what you're saying a little later and see if I can come up with anything helpful.

The test case that's failing however, is already pretty minimal. You can simplify it a bit further by removing the second UBO, but there's not much to it.

What happens is that you define a case class like this:

case class UBO1(TIME: highp[Float], val VIEWPORT_SIZE: vec2)

Then within the shader block you say:

ubo[UBO1]

And it outputs this:

layout (std140) uniform UBO1 {
  highp float TIME;
  vec2 VIEWPORT_SIZE;
};

@davesmith00000
Copy link
Member

@nicolasstucki Apologies for the slow response. I've gone ahead and made this my problem (probably was anyway!).

This PR makes Ultraviolet compatible with 3.4.2-RC1-bin-20240311-02c2a6e-NIGHTLY, and happily, works just fine with 3.4.1, too.

#111

So that will be in the next release, which will be soon.

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