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

Under -Xsource:3, adjust 2.13.9 change to ignore override type for whitebox macro expansion #10188

Merged
merged 1 commit into from Oct 26, 2022

Conversation

som-snytt
Copy link
Contributor

@som-snytt som-snytt commented Oct 13, 2022

Improve the condition for inferring the type of an overriding def under -Xsource:3: use the precise type of the RHS if the RHS is the expansion of a whitebox macro.

Adjusts #9891
Fixes scala/bug#12647

@scala-jenkins scala-jenkins added this to the 2.13.11 milestone Oct 13, 2022
@som-snytt
Copy link
Contributor Author

Probably the openMacros check can be removed.

//t.tpe.typeSymbol.isAnonOrRefinementClass && also note it doesn't matter what the narrower type is, but this is the use case.

@som-snytt som-snytt requested a review from lrytz October 13, 2022 17:51
Copy link
Member

@lrytz lrytz left a comment

Choose a reason for hiding this comment

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

LGTM otherwise, thank you! This fix makes sense 👍

*/
private def assignTypeToTree(tree: ValOrDefDef, defnTyper: Typer, pt: Type): Type = {
val rhsTpe = tree match {
case ddef: DefDef if tree.symbol.isTermMacro => defnTyper.computeMacroDefType(ddef, pt)
case ddef: DefDef if tree.symbol.isTermMacro => defnTyper.computeMacroDefType(ddef, pt) // unreached, see methodSig
Copy link
Member

Choose a reason for hiding this comment

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

So this can be removed?

There's also a comment in computeMacroDefType saying

if a macro def doesn't have explicitly specified return type, this method will be called again by assignTypeToTree

is that stale?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was reluctant to expand the scope, as macros remain experimental.

@SethTisue SethTisue added the release-notes worth highlighting in next release notes label Oct 19, 2022
@SethTisue
Copy link
Member

@lrytz want to press "merge", here?

@lrytz lrytz merged commit 8fa1e7c into scala:2.13.x Oct 26, 2022
@SethTisue SethTisue changed the title Ignore override type just for whitebox expansion Under -Xsource:3, adjust 2.13.10 change to ignore override type for whitebox macro expansion Oct 26, 2022
@SethTisue SethTisue changed the title Under -Xsource:3, adjust 2.13.10 change to ignore override type for whitebox macro expansion Under -Xsource:3, adjust 2.13.9 change to ignore override type for whitebox macro expansion Oct 26, 2022
@som-snytt som-snytt deleted the issue/12647-more-whitebox branch October 26, 2022 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes worth highlighting in next release notes
Projects
None yet
4 participants