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

Fix #12260: Add underscore to match type syntax #12261

Merged
merged 7 commits into from Jan 23, 2022

Conversation

OlivierBlanvillain
Copy link
Contributor

Also subsumes #9172 by fixing #9171.

@odersky
Copy link
Contributor

odersky commented Apr 29, 2021

simplify is very costly. So we do it once when we generate types. Why is it needed in type constant folding? We should try to avoid it there, or some code that constant folds a lot might blow up in complexity.

@OlivierBlanvillain
Copy link
Contributor Author

I opened #12278 to discuss simplify separately, it's orthogonal to this PR.

@OlivierBlanvillain OlivierBlanvillain marked this pull request as ready for review April 29, 2021 15:32
compiler/src/dotty/tools/dotc/parsing/Parsers.scala Outdated Show resolved Hide resolved
@@ -78,7 +78,7 @@ longer explanation available when compiling with `-explain`
| and cannot be shown to be disjoint from it either.
| Therefore, reduction cannot advance to the remaining case
|
| case _ => String
| case Any => String
Copy link
Contributor

Choose a reason for hiding this comment

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

I think those should be _ => String since that was what's written.

@OlivierBlanvillain
Copy link
Contributor Author

I updated the PR with another attempt, this time parsing case _ as Ident(_) and typing the corresponding tree as Any. I think it's overall much cleaner but we still lose the distinction between case _ and case Any in the error message...

@SethTisue
Copy link
Member

What's the status of case ??

If I understand correctly, there's three ways to write the same thing: case ?, case _, and case Any, and they are all intended to be synonymous? Is one of them considered preferred style?

In term-level matches, under -source:future I'm expected to write e.g. case xs: List[?] rather than case _: List[_], so this sets an expectation that the way to write "any type, I don't care" is ?, not _. If so, why support case _?

(I don't have any firm opinions about this, I'm just asking questions...)

@OlivierBlanvillain
Copy link
Contributor Author

If I understand correctly, there's three ways to write the same thing: case ?, case _, and case Any, and they are all intended to be synonymous?

Yes, they are 3 equivalant ways to write catch-all patterns for match types. The situation is actually symetrical between term and type level:

def foo(x) = x match
  case _ =>
  case _: Any =>
  case _: ? => // Unlike scalac which complains with
               // "error: unbound wildcard type",
               // Dotty is perfectly fine with this...
type Foo[X] = X match
  case _ =>
  case Any =>
  case ? =>

Is one of them considered preferred style?

I like case _ => for the symmetry, but it's just my personal preference :)

@som-snytt
Copy link
Contributor

Previous missed opportunity was to accept case if expr => and perhaps case =>. Now that seems less weird.

I sense a pun on "Casey at the Bat" coming on. "There is no joy in Matchville."

@dwijnand
Copy link
Member

dwijnand commented Jul 29, 2021

Yes, they are 3 equivalant ways to write catch-all patterns for match types. The situation is actually symetrical between term and type level:

Those aren't equivalent though: in the match expression it's switching from terms to types, while in match types it's types only. So I feel like we either embrace the symmetry with match expressions and make case _ => the "symbolic" catch all, or we embrace that ? as the symbol for 🤷🏼 and deprecate case _ =>. 3 options seems unnecessary to me.

@OlivierBlanvillain
Copy link
Contributor Author

The community build fails without the case ? => (it's used in scodecs). I propose to move that discussion to a separate issue.

@OlivierBlanvillain
Copy link
Contributor Author

rebased

@bishabosha
Copy link
Member

New issue that would be fixed by this: #13855

This is a hack to tell `case _ =>` and `case Any =>` apart while printing match
type error messages. At this point in the compilation pipeline both `pat` types
=:= Any, however because of the slight difference in representation we are able
to restore the original syntax. Here is what `pat.toString` gives in each case:

    // case _ =>
    (TypeRef(ThisType(TypeRef(NoPrefix,module class scala)),class Any))

    // case Any =>
    (TypeRef(TermRef(ThisType(TypeRef(NoPrefix,module class <root>)),object scala),class Any))

The second case doesn't `eq defn.AnyType` because it starts from <root>, and
thus doesn't match the pattern added in this commit.
@odersky odersky merged commit 2553b9e into scala:master Jan 23, 2022
@odersky odersky deleted the fix-12260 branch January 23, 2022 17:49
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 24, 2022
The warning was removed in scala#12261
nicolasstucki added a commit to dotty-staging/dotty that referenced this pull request Jan 24, 2022
The warning was removed in scala#12261

Fixes scala#14332
@nicolasstucki nicolasstucki mentioned this pull request Jan 24, 2022
olsdavis pushed a commit to olsdavis/dotty that referenced this pull request Apr 4, 2022
The warning was removed in scala#12261

Fixes scala#14332
@Kordyjan Kordyjan added this to the 3.1.2 milestone Aug 2, 2023
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.

Match type syntax under -source:future
7 participants