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

Ongoing Scala 3 support #280

Merged
merged 85 commits into from
Feb 24, 2022
Merged

Ongoing Scala 3 support #280

merged 85 commits into from
Feb 24, 2022

Conversation

jrudolph
Copy link
Collaborator

@jrudolph jrudolph commented Aug 30, 2021

Continuation of #274

This PR documents the ongoing progress on Scala 3 support. It stays open until the support is ready for merging into master.

The commits are supposed to stay, so let's try to use reasonable commits. Changes can be PR'd against this branch.

@jrudolph jrudolph mentioned this pull request Aug 30, 2021
6 tasks
@yanns yanns mentioned this pull request Aug 30, 2021
@jrudolph
Copy link
Collaborator Author

@nicolasstucki pattern matching calls to this method with quoted patterns seems quite hard because of the complex relationship of all the type parameters involved:

https://github.com/sirthias/parboiled2/blob/master/parboiled-core/src/main/scala/org/parboiled2/RuleDSLActions.scala#L30

It seems easy enough to parse them using trees/terms but is there any chance of matching them using quoted patterns?

E.g. the first problem is that you cannot even put in wildcard types for the type parameters because they don't fulfill the bounds:

case '{($p: Parser).capture[a, b]($arg)($d)} =>

fails with

[error]     |Type argument a does not conform to upper bound org.parboiled2.support.hlist.HList in subpart org.parboiled2.Rule[a, b] of inferred type scala.quoted.Expr[org.parboiled2.Rule[a, b]]

Should it work leaving off the type arguments completely? In that case, the macro compiles but it does not match the invocation when the macro is executed. Are there any extra tools to debug non-matching quote patterns?

@nicolasstucki
Copy link

If I understand correctly you need to add upper bounds to a and b. If that is the case then you can use

case '{ type a <: T; type b < T2; ($p: Parser).capture[`a`, `b`]($arg)($d)} =>

@jrudolph
Copy link
Collaborator Author

Cool, I'm going to try that.

@jrudolph
Copy link
Collaborator Author

If I understand correctly you need to add upper bounds to a and b. If that is the case then you can use

case '{ type a <: T; type b < T2; ($p: Parser).capture[`a`, `b`]($arg)($d)} =>

Something similar compiles but still doesn't match:

case '{ type I <: HList; type O <: HList; ($p: Parser).capture[I, O]($arg: Rule[I, O])($d) } =>

doesn't match this tree:

$anon.this.capture[org.parboiled2.support.hlist.HNil, org.parboiled2.support.hlist.HNil]($anon.this.ch('b'))(org.parboiled2.support.hlist.ops.hlist.Prepend.hnilPrepend1[org.parboiled2.support.hlist.HNil, org.parboiled2.support.hlist.::[scala.Predef.String, org.parboiled2.support.hlist.HNil]])

@jrudolph jrudolph closed this Aug 30, 2021
@jrudolph jrudolph reopened this Aug 30, 2021
@xuwei-k
Copy link
Collaborator

xuwei-k commented Nov 22, 2021

@yanns
Copy link
Contributor

yanns commented Jan 12, 2022

We're missing an implementation for runSubParser if anyone wants to pick that.

@liosedhel
Copy link

Hi ;) First of all, thanks for the effort towards migrating this lib! Is there any time schedule to migrate this lib (and hence akka-http) to Scala 3 in foreseeable future? Maybe there are some areas where one can quite easily contribute?

@jrudolph
Copy link
Collaborator Author

Hi ;) First of all, thanks for the effort towards migrating this lib! Is there any time schedule to migrate this lib (and hence akka-http) to Scala 3 in foreseeable future? Maybe there are some areas where one can quite easily contribute?

The biggest problem is that real world parsers still hang the Scala 3 compiler or fail with typing errors.

We have some tests commented out in ActionSpec that show this kind of behavior on a small scale, so you could try to pick one of those tests and see what is needed to make the tests pass. But this is by no means a simple task. This is territory where the old code worked because of un- or underdocumented features of the Scala 2 typing system and we will have to find our ways through to the un- or underdocumented or incompletely implemented features of the Scala 3 compiler.

@liosedhel
Copy link

liosedhel commented Feb 24, 2022

We have some tests commented out in ActionSpec

this now seems to be fixed, right? ;)
thank you @prolativ @KacperFKorban and of course @jrudolph for quick response and looking into it 🙏 Anything else here that one could help with? :)

@jrudolph jrudolph marked this pull request as ready for review February 24, 2022 10:02
Copy link
Collaborator Author

@jrudolph jrudolph left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my side this now done! Thanks everyone involved. @sirthias / @xuwei-k what would it take to do a new release?

@sirthias
Copy link
Owner

Awesome! Huge thanks again to everyone involved in this massive task!
I'll package and publish a new release right away...

@sirthias sirthias merged commit 35db004 into master Feb 24, 2022
@sirthias
Copy link
Owner

Ok, just released parboiled2 2.4.0, cross-built against:

  • 2.12.5
  • 2.13.8
  • 3.1.1

X

  • Java 8
  • scala-js 1.9.0

Unfortunately I had to (temporarily) disable the cross-build against scala-native due to a missing dependency (scala-check for scala-native 0.4.3) and weird binary-incompatible version of NIR errors, that I didn't have time to debug today.

Thank you again, everyone, for the awesome feat of pulling off this migration!

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.