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

Add customizable names for definitions in quotes #7346

Merged
merged 3 commits into from
Oct 8, 2019

Conversation

nicolasstucki
Copy link
Contributor

This allow naming val/def in quoted code with computed names

@nicolasstucki nicolasstucki changed the title Add spliced names Add customizable names for definitions in quotes Oct 4, 2019
This allow naming val/def in quoted code with computed names
Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

Otherwise, LGTM

docs/docs/reference/metaprogramming/macros.md Outdated Show resolved Hide resolved
docs/docs/reference/metaprogramming/macros.md Outdated Show resolved Hide resolved
docs/docs/reference/metaprogramming/macros.md Outdated Show resolved Hide resolved
docs/docs/reference/metaprogramming/macros.md Outdated Show resolved Hide resolved
* myVal + 1
* ```
*/
class showName(name: String) extends scala.annotation.Annotation
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: what about displayName or debugName? showName feels more like a verb.

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 chose show name because only the Expr.show will display it. Compiler debugging options as -Xprint will just print the annotations.

library/src/scala/tasty/reflect/Printers.scala Outdated Show resolved Hide resolved
}
def powerCodeD(n: Long, i: Int, x: Expr[Double])(given QuoteContext): Expr[Double] =
if (n == 0) '{1.0}
else if (n % 2 == 0) '{ @showName(${Expr("a" + i)}) val y = $x * $x; ${powerCodeD(n / 2, idx * 2, 'y)} }
Copy link
Contributor

@julienrf julienrf Oct 7, 2019

Choose a reason for hiding this comment

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

Should we instead provide a means to developers for generating val definitions?

Suggested change
else if (n % 2 == 0) '{ @showName(${Expr("a" + i)}) val y = $x * $x; ${powerCodeD(n / 2, idx * 2, 'y)} }
else if (n % 2 == 0) {
val y = named("a" + i, '{ $x * $x})
'{ $y; ${powerCodeD(n / 2, idx * 2, y)} }
}

Also, I’m very surprised that there is no name clash in the expanded code you shown. It seems that all the lines of the block are in the same lexical scope.

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, I’m not sure about the suggestion above. It can not express something like '{ val MyExtractor(y) = $x * $x }, for instance.
Otherwise, could we support something like the following?

Suggested change
else if (n % 2 == 0) '{ @showName(${Expr("a" + i)}) val y = $x * $x; ${powerCodeD(n / 2, idx * 2, 'y)} }
else if (n % 2 == 0) {
val y = name("a" + i)
'{ val $y = $x * $x; ${powerCodeD(n / 2, idx * 2, y)} }
}

I believe we can’t, though…

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The annotation is the first step towards splicing a name. This PR is a starting point to support all those patterns. Ideally a '{ val $y = ... } will be desugared to '{ @showName(${Expr(y.toString)}) val y = ... }

@nicolasstucki
Copy link
Contributor Author

@liufengyun I extracted the second part of this PR in #7388. This PR only contains the core/internal logic required to splice names in quotes.

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

LGTM 👍

I'm happy to see showName now is in scala.internal.

@nicolasstucki nicolasstucki merged commit 45b6ebd into scala:master Oct 8, 2019
@nicolasstucki nicolasstucki deleted the spliced-names branch October 8, 2019 18:00
@anatoliykmetyuk anatoliykmetyuk added this to the 0.20 Tech Preview milestone Oct 31, 2019
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.

4 participants