Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Refutable extractor may be an Apply tree #15651

Merged
merged 1 commit into from
Jul 13, 2022
Merged

Conversation

griggt
Copy link
Contributor

@griggt griggt commented Jul 12, 2022

Fixes #15650

@griggt griggt added the backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. label Jul 12, 2022
@griggt griggt added this to the 3.2.0 backports milestone Jul 12, 2022
@griggt griggt requested a review from bishabosha July 12, 2022 01:22
Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

Let's avoid MatchErrors outright, by using the recursive funPart, and falling back if it's not a select.

Comment on lines 833 to 837
fn match
case Select(id, _) => id
case Apply(Select(id, _), _) => id
case TypeApply(Select(id, _), _) => id
em"pattern binding uses refutable extractor `$extractor`"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
fn match
case Select(id, _) => id
case Apply(Select(id, _), _) => id
case TypeApply(Select(id, _), _) => id
em"pattern binding uses refutable extractor `$extractor`"
tpd.funPart(fn) match
case Select(id, _) => id
case _ => ""
if extractor == ""
em"pattern binding uses refutable extractor"
else
em"pattern binding uses refutable extractor `$extractor`"

Copy link
Contributor Author

@griggt griggt Jul 13, 2022

Choose a reason for hiding this comment

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

oooh, was unaware of funPart, very nice, thanks!

Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

extracting the extractor name needs to deal with arbitrary parameter lists (and type parameter lists for future proofing), for example it still crashes if I add another using clause:

class Rational

import scala.quoted.*

class TC

object meta:
  object rationalTE:
    def unapply(using Quotes)(using TC)(tr: quotes.reflect.TypeRepr): Option[Rational] = ???

  def foo(using Quotes)(using TC)(p: quotes.reflect.TypeRepr): Unit =
    val rationalTE(e) = p  // warn: pattern binding uses refutable extractor `meta.rationalTE`

@dwijnand
Copy link
Member

dwijnand commented Jul 12, 2022

Yeah, using TreeInfo's funPart will address that.

And in fact we need to be able to handle arbitrary parameter lists (and type
parameter lists) when extracting the extractor name.

Fixes scala#15650
@griggt
Copy link
Contributor Author

griggt commented Jul 13, 2022

Rebased with suggested changes and an additional test case.

@griggt griggt requested review from bishabosha and dwijnand July 13, 2022 05:29
@bishabosha bishabosha merged commit af16d56 into scala:main Jul 13, 2022
@griggt griggt deleted the fix-15650 branch July 13, 2022 15:10
@griggt griggt added backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" and removed backport:nominated If we agree to backport this PR, replace this tag with "backport:accepted", otherwise delete it. labels Jul 13, 2022
@Kordyjan Kordyjan added backport:done This PR was successfully backported. and removed backport:accepted This PR needs to be backported, once it's been backported replace this tag by "backport:done" labels Jul 26, 2022
@Kordyjan Kordyjan modified the milestones: 3.2.0 backports, 3.2.1 Aug 1, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
backport:done This PR was successfully backported.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MatchError when using refutable extractor
4 participants