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 3 Support (1000USD Bounty) #2

Closed
lihaoyi opened this issue Jan 1, 2024 · 11 comments · Fixed by #11
Closed

Scala 3 Support (1000USD Bounty) #2

lihaoyi opened this issue Jan 1, 2024 · 11 comments · Fixed by #11

Comments

@lihaoyi
Copy link
Member

lihaoyi commented Jan 1, 2024

Currently ScalaSql only supports Scala 2.13. Getting it working on Scala 3 would involve porting the table macros, which is one ~100 line file https://github.com/com-lihaoyi/scalasql/blob/main/scalasql/query/src-2/TableMacro.scala. I don't have the knowledge of Scala 3 macros to do it myself, but it shouldn't be hard to port, and the rest of the ScalaSql library should "just work".

To incentivize contribution, I'm putting a 1000USD bounty on resolving this ticket to the first person who can get all the ScalaSql tests passing on Scala 3. This is payable via bank transfer, and at my discretion in case of ambiguity.

@lihaoyi
Copy link
Member Author

lihaoyi commented Jan 1, 2024

@KuceraMartin said he wanted to look into this, but if anyone else wants to try their hand at this feel free to do so

@KuceraMartin KuceraMartin mentioned this issue Jan 7, 2024
5 tasks
@ghik
Copy link

ghik commented Jan 18, 2024

Sounds like a fun thing to do

@ghik
Copy link

ghik commented Jan 18, 2024

I tried doing this and discovered scala/scala3#19480

@KuceraMartin
Copy link
Collaborator

KuceraMartin commented Jan 19, 2024

I discovered scala/scala3#19436 in #3. I also tried a different approach with mirrors and ran into another compiler crash scala/scala3#19493.

It seems there are many different ways to approach this. @ghik would you be interested in discussing our thoughts? Maybe together we could find another approach that doesn't break the compiler :D

@ghik
Copy link

ghik commented Jan 19, 2024

Sure, but I haven't even gotten to porting the macro itself. I just tried to compile tests on Scala 3...

@KuceraMartin
Copy link
Collaborator

Actually I never tried compiling all the tests. Now that I did, I see I'm getting the same compiler crash as you.

I started by removing most tests and trying to write a macro that would generate something similar to what the Scala 2 TableMacros does, as per @lihaoyi's advice. You could try the same thing if you want. If you do so, you can cherry-pick this commit a96df75 which fixes some trivial Scala 3 incompatibilities outside of macros. Then, delete all tests except for e.g. Main.scala. Then, if you just replace TableMacros with

trait TableMacros:
   given metadata[V[_[_]]]: Table.Metadata[V] = ???

it will at least compile. Later on, you can start adding some tests back. I tried multiple and never ran into that compiler crash. For instance SqliteExample.scala should work.

If you're interested in discussing it further, let's connect on Discord (mine is martin.kucera)

@ghik
Copy link

ghik commented Jan 21, 2024

@KuceraMartin FYI scala/scala3#19480 already has a working fix on the way, I compiled the tests successfully with it

I'm going to take a break from this for now, but I'll be happy to discuss details if I come back to it.

@lihaoyi lihaoyi changed the title Scala 3 Support (500USD Bounty) Scala 3 Support (1000USD Bounty) May 14, 2024
@vvidlearn
Copy link

vvidlearn commented May 15, 2024

Hi this sounds like an interesting exercise. I am new to macros but consider this as an opportunity to learn them. So would like to work on it if its ok. If this will be too much of an effort for a macro newbie I am happy for someone else to work on this.Thank you.

@lihaoyi
Copy link
Member Author

lihaoyi commented May 15, 2024

Hi this sounds like an interesting exercise. I am new to macros but consider this as an opportunity to learn them. So would like to work on it if its ok. Thank you.

Please go ahead! No need to ask me for permission

@mrdziuban
Copy link
Contributor

Hey @vvidlearn, are you still looking into this? I'm curious to look as well but don't want to step on your toes

lihaoyi pushed a commit that referenced this issue May 25, 2024
Closes #2 

## Macros

Just like @KuceraMartin in #3, I ran into scala/scala3#19493 and
scala/scala3#19436 when trying to resolve a `TypeMapper` by importing
from a `DialectTypeMappers`. As a workaround, I introduced [additional
`implicit def`s in the `TableMapper` companion
object](https://github.com/com-lihaoyi/scalasql/blob/a7d6c531bf7b9cc2f5e2c175906d2a1e961de206/scalasql/core/src/TypeMapper.scala#L58-L121)
that instead rely on an implicit instance of `DialectTypeMappers`, i.e.
in a macro:

```scala
// bad, causes a compiler crash
// TableMacro.scala
(dialect: DialectTypeMappers) => {
  import dialect.*
  summonInline[TypeMapper[t]]
}

// good
// TypeMapper.scala
implicit def stringFromDialectTypeMappers(implicit d: DialectTypeMappers): TypeMapper[String] = d.StringType

// TableMacro.scala
(dialect: DialectTypeMappers) => {
  given d: DialectTypeMappers = dialect
  summonInline[TypeMapper[t]]
}
```

## Supporting changes

In addition to building out the macros in Scala 3, the following changes
were necessary:

1. Update the generated code to ensure `def`s aren't too far to the left
-- this is to silence Scala 3 warnings
2. Convert `CharSequence`s to `String`s explicitly -- see the [error the
Scala 3 compiler reported
here](9ffeb06)
3. Remove `try` block without a corresponding `catch` -- see the
[warning the Scala 3 compiler reported
here](011c3f6)
4. Add types to implicit definitions
5. Mark `renderSql` as `private[scalasql]` instead of `protected` -- see
the [error the Scala 3 compiler reported
here](8e767e3)
6. Use Scala 3.4 -- this is a little unfortunate since it's not the LTS
but it's necessary for the Scala 3 macros to [match on higher kinded
types like
this](https://github.com/com-lihaoyi/scalasql/blob/a7d6c531bf7b9cc2f5e2c175906d2a1e961de206/scalasql/query/src-3/TableMacro.scala#L48-L52).
This type of match doesn't work in Scala 3.3
7. Replace `_` wildcards with `?` -- this is to silence Scala 3 warnings
8. Replace `Foo with Bar` in types with `Foo & Bar` -- this is to
silence Scala 3 warnings
9. Add the `-Xsource:3` compiler option for Scala 2 -- this is necessary
to use the language features mentioned in points 7 and 8
10. Add a number of type annotations to method overrides -- this is to
silence warnings reported by the Scala 2 compiler as a result of
enabling `-Xsource:3`. All of the warnings relate to the inferred type
of the method changing between Scala 2 and 3
@vvidlearn
Copy link

hi @mrdziuban sorry for the late reply, I am just a macro beginner. So it was beyond me. glad you completed it.

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 a pull request may close this issue.

5 participants