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

scala3: incomplete support for _ in types #2807

Closed
kitbellew opened this issue Aug 19, 2022 · 14 comments
Closed

scala3: incomplete support for _ in types #2807

kitbellew opened this issue Aug 19, 2022 · 14 comments
Assignees

Comments

@kitbellew
Copy link
Contributor

kitbellew commented Aug 19, 2022

https://dotty.epfl.ch/docs/reference/changed-features/wildcards.html describes this:

  • scala 2:
    • type wildcard is _
    • type placeholder is unsupported
  • scala 3.0:
    • type wildcard is ? and _
    • type placeholder is *
  • scala 3.1:
    • type wildcard is ?, _ is deprecated
    • type placeholder is *
  • scala 3.2:
    • type wildcard is ?
    • type placeholder is _, * is deprecated
  • scala 3.3:
    • type wildcard is ?
    • type placeholder is _

In Dialect, we have the following today:

  • allowQuestionMarkPlaceholder (which is misleading since it's a wildcard but not a placeholder)
  • allowPlusMinusUnderscoreAsPlaceholder (which is a bit misleading since should apply to invariant placeholder types as well, not just covariant/contravariant)
  • allowTypeParamUnderscore (not sure if this is for a different syntax element and could remain as-is)

I would suggest the following Dialect changes (and corresponding parser modifications):

  • rename allowQuestionMarkPlaceholder to allowQuestionMarkAsTypeWildcard
    • keep withAllowQuestionMarkPlaceholder method for now but deprecate it (make it redirect to the new withAllowQuestionMarkAsTypeWildcard)
  • similarly, rename allowPlusMinusUnderscoreAsPlaceholder to allowUnderscoreAsTypePlaceholder (to distinguish _ between wildcard in 3.1 and placeholder in 3.2)
  • possibly also add allowStarAsTypePlaceholder
    • otherwise, allow star if allowQuestionMarkAsTypeWildcard=true and allowUnderscoreAsTypePlaceholder=false, although this doesn't cover the 3.2 case
@kitbellew
Copy link
Contributor Author

kitbellew commented Aug 19, 2022

also:

  • Type.Placeholder is used for wildcard and might need to be split
  • Type.Placeholder is used instead of a pattern tree (see ConvertToNewScala3Syntax bugfix: _ only in types scalafmt#3303)
    • this is part of a larger issue that regular Type classes are used for type match patterns (whereas Pat is used for Term match patterns)
    • perhaps a whole new set of TypePat classes should be introduced, and parsing changed, to reflect that

@dos65
Copy link
Member

dos65 commented Aug 22, 2022

@kitbellew Thanks for collecting the info! Looks reasonable.

@tgodzik
Copy link
Collaborator

tgodzik commented Aug 23, 2022

I really hoped we wouldn't need to create a new dialect for each minor version of Scala 3, which will be super difficult for people to handle :/

I wonder if we could just allow each placeholder in every place, from the parser perspective this wouldn't be much of an issue:

type wildcard is ? and _
type placeholder is * and _

Is there any specific issue that it would cause issues with? Probably the rewrite rules? 🤔

@tgodzik
Copy link
Collaborator

tgodzik commented Aug 23, 2022

Also it seems the 3.2 is not bringing about the advertised changes, _ is still available as type placeholder.

We can postpone the changes here.

@kitbellew
Copy link
Contributor Author

I wonder if we could just allow each placeholder in every place, from the parser perspective this wouldn't be much of an issue:

type wildcard is ? and _ type placeholder is * and _

interesting idea... as it stands currently, our parser can tell that F[_ :> lo] is a wildcard with bounds but when it comes to the simple F[_], it can't yet. perhaps we need to provide it a parsing context flag which will allow it to tell the difference. frankly, i don't know when each is applicable (placeholder vs wildcard)...

Is there any specific issue that it would cause issues with? Probably the rewrite rules? 🤔

@tgodzik
Copy link
Collaborator

tgodzik commented Aug 23, 2022

I think it should be possible to tell from the context. Though that depends on where type lambdas can be defined 🤔

Also, how would we want to represent type lambda with placeholder? Something like:

Type.Lambda(List(Type.Placeholder()), Type.Appl(Type.Name("F"), List(Type.Placeholder()))

or would we want to pack it in an additional Type.AnonymousFunction ?

Anyway, we don't have to do it yet, since the migration is still far off, but it's good to start the conversation for sure. We might need to end up with additional dialects, but I don't love that idea since we would have much more of them than even in case of Scala 2. Which is why if possible I would love to see the parser try to work with all of the placeholders possible.

@kitbellew
Copy link
Contributor Author

something like that, yes. is there any documentation on lambdas and where there can be used?

@tgodzik
Copy link
Collaborator

tgodzik commented Aug 23, 2022

Very limited is here:

https://docs.scala-lang.org/scala3/reference/new-types/type-lambdas.html

but looking into syntax description it seems it might be used anywhere a type can be used :/ so my idea might not work unfortunately.

@kitbellew
Copy link
Contributor Author

@tgodzik the wildcards document mentions this:

It also removes the wart that, used as a type parameter, F[_] means F is a type constructor whereas used as a type, F[_] means it is a wildcard (i.e. existential) type. In the future, F[_] will mean the same thing, no matter where it is used.

Does that mean that we shouldn't use context-aware parsing which might result in a different interpretation?

@tgodzik
Copy link
Collaborator

tgodzik commented Aug 25, 2022

Yeah, accepting all possibilities will not be correct 🤔 We might need to do different dialects then after the change is introduced.

@kitbellew
Copy link
Contributor Author

Yeah, accepting all possibilities will not be correct 🤔 We might need to do different dialects then after the change is introduced.

@tgodzik , in that case, would it make sense first to introduce the allow flags that i have proposed (#2811 and later PRs), and interpret _ or * based on that, and then later possibly introduce additional validation to make sure that take context into account to ensure that the placeholder or wildcard are actually meaningful?

if so, i can modify/relax the dialect definitions (#2809) to reflect that some things have not yet been done as mentioned in the docs.

@tgodzik
Copy link
Collaborator

tgodzik commented Aug 26, 2022

@tgodzik , in that case, would it make sense first to introduce the allow flags that i have proposed (#2811 and later PRs), and interpret _ or * based on that, and then later possibly introduce additional validation to make sure that take context into account to ensure that the placeholder or wildcard are actually meaningful?

Sure, it makes sense to introduce the flags, but do we need the context actually if we get ? and _ to each mean one thing only?

@kitbellew
Copy link
Contributor Author

@tgodzik , in that case, would it make sense first to introduce the allow flags that i have proposed (#2811 and later PRs), and interpret _ or * based on that, and then later possibly introduce additional validation to make sure that take context into account to ensure that the placeholder or wildcard are actually meaningful?

Sure, it makes sense to introduce the flags, but do we need the context actually if we get ? and _ to each mean one thing only?

depends on how careful we want to be. since they can mean one thing only but not always acceptable, we can either reject them in inappropriate context or assume the compiler would do that...

@tgodzik
Copy link
Collaborator

tgodzik commented Aug 26, 2022

I think it's fine to leave it up to the compiler, we don't always reject all the problematic syntax, which is fine, we can be more inclusive.

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

3 participants