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

Disallow phase inconsistent inline parameters #8061

Conversation

nicolasstucki
Copy link
Contributor

@nicolasstucki nicolasstucki commented Jan 22, 2020

In preparation for #8060.

Originally we used inline parameters as the way to recover statically known values from the arguments. This worked on some predefined types such as literal primitives, case objects, case classes, enums among others. The latter ones where ad-hoc addons without a clear specification.

ValueOfExpr also recovers statically known values but it is more general. It can be implemented for any arbitrary type that represents a value. In this PR we introduce some extra ways to use this such as Expr.value, matching.Value and matching.ValueSeq.

ValueOfExpr makes the current use of inline parameters completely redundant hence this PR replaces all uses of inline parameters by equivalent code using ValueOfExpr.

It also simplifies the PCP as it removes a special case from the rules.

@nicolasstucki nicolasstucki self-assigned this Jan 22, 2020
@nicolasstucki nicolasstucki force-pushed the disallow-phase-inconsistent-inline-params branch 10 times, most recently from 2ecdf91 to b937f9f Compare January 23, 2020 07:17
@nicolasstucki nicolasstucki force-pushed the disallow-phase-inconsistent-inline-params branch 5 times, most recently from 334930e to 07f104e Compare January 23, 2020 08:54
@nicolasstucki nicolasstucki marked this pull request as ready for review January 23, 2020 09:46
@nicolasstucki
Copy link
Contributor Author

Will need to be rebased on #8077 (the second commit will disapear)

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
inline def power(x: Double, inline n: Int) = ${ powerCode('x, 'n) }

private def powerCode(x: Expr[Double], n: Expr[Int]): Expr[Double] =
powerCode(x, n.value) // Transform `n` into an Int or emit an error if it is not possible
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe do it safely by n.getValue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This has the same semantics as before. If the inline parameter is not a constant, an error is emitted.

library/src/scala/quoted/ValueOfExpr.scala Show resolved Hide resolved
def unapply[T](expr: Expr[T])(given valueOf: ValueOfExpr[T], qxtc: QuoteContext): Option[T] =
valueOf(expr)

}
Copy link
Contributor

Choose a reason for hiding this comment

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

We already have extractor for Const, a duplicate? The name Value is a little misleading, if we in fact only want to get the literal const out.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, Value is more general. I plan to replace and remove Const in another PR

* Otherwise returns the value.
*/
final def value[U >: T](given qctx: QuoteContext, valueOf: ValueOfExpr[U]): U =
valueOf(this).getOrElse(qctx.throwError(s"Expected a known value. \n\nThe value of: $show\ncould not be recovered using $valueOf", this))
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems this method only invites errors in meta-programming, maybe we should just remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Without it we would end up writing expr.getValue.get all over the place which would crash the macro expansion. This alternative emits an error saying that the call expected a constant value. Pointing to the argument that is not a value.

@@ -18,6 +18,14 @@ class Expr[+T] private[scala] {
*/
final def getValue[U >: T](given qctx: QuoteContext, valueOf: ValueOfExpr[U]): Option[U] = valueOf(this)
Copy link
Contributor

Choose a reason for hiding this comment

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

I find getValue is a little misleading, if what it does is to get the literal out. Maybe getLiteralValue?

Copy link
Contributor Author

@nicolasstucki nicolasstucki Jan 23, 2020

Choose a reason for hiding this comment

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

It is not necessarily a literal value.

@nicolasstucki nicolasstucki force-pushed the disallow-phase-inconsistent-inline-params branch 3 times, most recently from 4e68df9 to e90f44d Compare January 23, 2020 15:07
@nicolasstucki nicolasstucki force-pushed the disallow-phase-inconsistent-inline-params branch from e90f44d to e56574f Compare January 23, 2020 15:49
@nicolasstucki nicolasstucki merged commit 9982f0d into scala:master Jan 23, 2020
@nicolasstucki nicolasstucki deleted the disallow-phase-inconsistent-inline-params branch January 23, 2020 16:33
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.

None yet

2 participants