-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Change syntax of splices and quotes #5918
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
Change syntax of splices and quotes #5918
Conversation
There are some tests that I could not fix. @nicolasstucki, can you have a look at these? |
7fed7c9
to
0caecac
Compare
5421c2a
to
4b722bc
Compare
Rebased |
I accidentally pushed the wrong scalatest submodule in one of my commits. The last commit fixes that. |
accb9ed
to
2ca5297
Compare
5536a54
to
63471fe
Compare
@nicolasstucki Why did you need to revert the scalatest community build? That does not work, as it now fails with an illegal comparison. |
I will re-revert it. I now noticed that my issue was due to a dirty build. |
I'll do it as part of another push. |
e4ff734
to
e21dce5
Compare
So we now have quoted idents as well: |
21c62df
to
0a418b0
Compare
Finally, tests pass again |
Quote(t) | ||
} | ||
else { | ||
migrationWarningOrError(em"""symbol literal '${in.name} is no longer supported, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We currently only support 'x
, 'true
, 'false
and 'null
within a '{ ... }
or ${ ... }
. Is there any reason not to support them outside when isScala2Mode == false
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also consider reintroducing the syntax '()
for quoted unit. It look more natural than '{}
f543d04
to
f940d00
Compare
Looks good, but needs an extra reviewer as I added lots of commits. @biboudis will also review it. |
f940d00
to
33a53a7
Compare
Rebased |
- '$' is no longer a keyword. - 'x is treated as a quote instead of a symbol literal inside splices - 'x and $x are single tokens (of kind QUOTEID and IDENTIFIER) This reverts part of commit 8cfa1fb.
Correctly handle 'this, 'true, 'false, 'null.
Adapt splice syntax and modify tests to no cancel quotes and splices before the checks
305e3bf
to
b560392
Compare
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks all good, except for the question what to name splice
. But we can still change that later.
I already approved once, so it seems the system does not let me approve once more. But I do. |
override def isTerm: Boolean = op.name.isTermName | ||
override def isType: Boolean = op.name.isTypeName | ||
override def isTerm: Boolean = op.isTerm | ||
override def isType: Boolean = op.isType | ||
} | ||
|
||
/** A typed subtree of an untyped tree needs to be wrapped in a TypedSplice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Super nitpick 🙈, TypSplice
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this is actually a TypeSplice
which is a completely different concept 🙉. That one wraps a tree that is typed inside an untyped tree.
In this version of `reflect`, the type of `x` is now the result of | ||
splicing the `Type` value `t`. This operation _is_ splice correct -- there | ||
is one quote and one splice between the use of `t` and its definition. | ||
|
||
To avoid clutter, the Scala implementation tries to convert any phase-incorrect | ||
reference to a type `T` to a type-splice, by rewriting `T` to `~implicitly[Type[T]]`. | ||
reference to a type `T` to a type-splice, by rewriting `T` to `${ the[Type[T]] }`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW ${ the[Type[T]] }
is surprisingly pleasant to read
Quote:
Splice: