Remove quoted.Const.unapply

[nicolasstucki]

This extractor was replaced with the more versatile extractor quoted.Expr.unapply.
Usually, it is better to use expr.unlift/expr.unliftOrError when it is simpler to do so.

• Remove definition of quoted.Const.unapply
• Replace uses of Const with Expr

Migration

• Consider using expr.unlift and them match on Option
• Consider using expr.unliftOrError
• Replace Const(x) with Expr(x) if the type is known
• Replace Const(x: Int) with '{ ${Expr(x)}: Int } if type of expression must be checked at runtime

[liufengyun]

What about keep both?

In meta-programming, Const is more familiar to users and will be frequently used.

[nicolasstucki]

There is some redundancy that should be avoided. This extractor can easily be added in a separate library or in a future version of the library.

There are two kinds of uses that seem to repeat:

• Use it to convert a single Expr[T] into a T. For this use case, it looks like using expr.unlift is much simpler.
• Use nested in a quoted pattern. The most used version seems to be nestled in a Varargs, maybe we should have a specialized variant for this one.

We also want to avoid conceptual confusions between Cont and reflect.Constant.

[liufengyun]

There is some redundancy that should be avoided. This extractor can easily be added in a separate library or in a future version of the library.

The redundancy is indeed a concern. Looking at the changes in tests, it seems to me Const is more declarative and straight-forward. Users can also get used to Unlifted, but it is not as nice as Const.

Maybe we teach users to use .unlift in non-patmat code, and Const(...) in patmat code?

[nicolasstucki]

Yes, we need to teach users to use .unlift.

We cannot keep Const and remove Unlifted as the first one is less expressive. We cannot rename Unlifted to Const as it would be extremely confusing if Const returns non-constants.

[liufengyun]

We cannot keep Const and remove Unlifted as the first one is less expressive. We cannot rename Unlifted to Const as it would be extremely confusing if Const returns non-constants.

If 99.99% usage of Unlifted in patmat is Const, it seems reasonable to keep Const and remove Unlifted. We don't loose expressiveness, as users can always use .unlift, which is more natural for complex usage that are not constants.

[nicolasstucki]

Const cannot replace Unlifted. It is less expressive.

We cannot use .unlift instead of Unlifted as this is mostly used in pattern position nested in a pattern ${}.

[nicolasstucki]

Short story, Const is like any of the other utilities that we have removed so far, it is a partial duplication of functionality that gives a second way to achieve something. All these can be added in a library to avoid overloading the cognitive load of the std-library.

[nicolasstucki]

We cannot keep Const and remove Unlifted as the first one is less expressive. We cannot rename Unlifted to Const as it would be extremely confusing if Const returns non-constants.

If 99.99% usage of Unlifted in patmat is Const, it seems reasonable to keep Const and remove Unlifted. We don't loose expressiveness, as users can always use .unlift, which is more natural for complex usage that are not constants.

Where did you get the 99.99%? I don't see any legitimate use of Const except for case Varargs(Consts(args)). Not that the tests were written before we had .unlift and use Const/Unlifted where they should not.

[nicolasstucki]

And most uses of Varargs(Consts(args)) are to extract the strings from StringContext

expr match {
      case '{ StringContext(${Varargs(Consts(parts))}: _*) } =>

but this value can be unlifted directly

val parts = expr.unliftOrError.parts

[liufengyun]

Where did you get the 99.99%? I don't see any legitimate use of Const except for case Varargs(Consts(args)). Not that the tests were written before we had .unlift and use Const/Unlifted where they should not.

I just want to use the conditional to emphasize there could be a valid argument for a different design other than the argument of expressiveness.

However, just did a quick search under tests/, we only get three instances of Unlifted, which IMHO might be cleaner to use .unlift instead.

For Consts, I agree that it can be removed. For Const, I am not so sure from the perspective of usability.

[nicolasstucki]

Which one would be better with Unlifted instead of .unlift?

[liufengyun]

Which one would be better with Unlifted instead of .unlift?

I propose to use .unlift to replace Unlifted, not the reverse.

[nicolasstucki]

That is exactly what I want, but the following statement seemed to indicate that there where some that should be kept as Unlifted

However, just did a quick search under tests/, we only get three instances of Unlifted, which IMHO might be cleaner to use .unlift instead.

Did I misunderstand?

[liufengyun]

That is exactly what I want, but the following statement seemed to indicate that there where some that should be kept as Unlifted

However, just did a quick search under tests/, we only get three instances of Unlifted, which IMHO might be cleaner to use .unlift instead.

Did I misunderstand?

I should remove However, pardon my English ;)

[nicolasstucki]

So we agree that in general the extractors should be avoided? Uses of Unlifted should be mostly in implementations of Unliftable.

[nicolasstucki]

Good. That is exactly what happened to the code in the community build.

[nicolasstucki]

I updated the first comment to reflect the point that it is better to not use extractors.

[liufengyun]

Good. That is exactly what happened to the code in the community build.

Yes, the changes in the CB look good. It's good to use .unlift in non-patmat code. What I propose is to keep Const for usage in patmat, WDYT?

[nicolasstucki]

I would not leave Const as most of it's uses look like missuses. Either it would have been an .unlift or an Unlifted within an Unliftable.

I would rather add it to the old quoted-util library that I will revive, update and publish as soon as I finish with the changes in the stdlib. In that case users would be able to use the same Const with the following import import scala.quoted.util._. Everything defined in quoted-util would be a potential addition to the scala.quoted if we see wide adoption of one of those helpers. This way we do not need to maintain it for binary compatibility if no one uses it.

[nicolasstucki]

Const would fit perfectly here https://github.com/nicolasstucki/quoted-util/tree/master/src/main/scala/quoted/util

[liufengyun]

@nicolasstucki IIRC, there are two issues with replacing Const(s) with Unlift(s) in patterns:

1. the usability issue: Unlift(s) doesn't look as nicely as Const(s) in patmat code.

2. the semantic issue: Cons(s) only matches a literal, while Unlift(s) can do evaluation.

[nicolasstucki]

This PR takes care of the semantics. We could try to rework the name if we find a better one in a following PR.

[liufengyun]

This PR takes care of the semantics. We could try to rework the name if we find a better one in a following PR.

Could you be more specific about this? It seems to me that if Const and Unlifted have different semantics, replacing the former with the latter in programs is problematic.

[nicolasstucki]

Unliftable covers all cases of Const where the type of the expression is known. The only thing that Const can do is match on an untyped expression. For example match on an Expr[Any] and get an Any constant out of it or some other arbitrary type. I'm not 100% sure that this use case is sound. This lack of typing is what we usually leave for the reflection API. This kind of code is found in a single tests that happens to have code that is unsound.

[liufengyun]

Unliftable covers all cases of Const where the type of the expression is known. The only thing that Const can do is match on an untyped expression. For example match on an Expr[Any] and get an Any constant out of it or some other arbitrary type. I'm not 100% sure that this use case is sound. This lack of typing is what we usually leave for the reflection API. This kind of code is found in a single tests that happens to have code that is unsound.

I checked the documentation of Unliftable:

/** A type class for types that can be turned from a `quoted.Expr[T]` to a `T` */
trait Unliftable[T] { ... }

From the documentaion above, we can see that Const has different semantics from Unliftable: Const should match a literal in the code, while Unliftable can perform evaluation to get a value of the type T.

[nicolasstucki]

The documentation of both should match. They were just written at different times and were never synced.

Unliftable should not perform an evaluation of the expression. The documentation specifically does not say that. The doc does need to be improved to make it clear. Unlifted and Const perform the same uplifting of the values, Unlifted extend this to non-primitives. In both cases, there is some evaluation but it is only a trivial evaluation that takes a constructor into its value.

• Unlifting a primitive value (Unlifted/Const): Take an expression containing a literal constant and construct its value.
• Unlifting constructors (only Unlifted for things like Option, List, ...): Take an expression containing a literal call to the constructor of the type and construct its value.

[liufengyun]

Unliftable should not perform an evaluation of the expression. The documentation specifically does not say that.

The subtle issue here is that a literal reading of Unliftable permits evaluation to get a value of T from Expr[T]. IIRC, Unliftabble should allow this for expressiveness, as there might be use cases where such evaluation is needed.

In contrast, a literal reading of Const(x: Int) disallows evaluation of Expr[Int] to get a value of Int.

[liufengyun]

It seems to me, a valid argument to remove Const.unapply is that in all/most of its usage, it should allow evaluation of the expression to get a constant value (e.g. advanced const-fold), while the literal semantics of Const disallow evaluation. This creates a gap.

By removing Const.unapply and preferring .unlift, meta-programs will be more robust.

The argument above depends on whether all usage of Const indeed permits evaluation.

[nicolasstucki]

Non of the extractors allow evaluation in the general sense. They only perform a minimal evaluation that take direct value construction into values, such as evaluate '{1} into 1. I'm adding some docs to Unliftable to make this clearer.