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

Change syntax of splices and quotes #5918

Merged
merged 26 commits into from Feb 22, 2019

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 14, 2019

Quote:

'{ ... }
'[...]

Splice:

${...}
$id

@odersky
Copy link
Contributor Author

odersky commented Feb 14, 2019

There are some tests that I could not fix. @nicolasstucki, can you have a look at these?

@nicolasstucki nicolasstucki force-pushed the change-splice-dollar branch 2 times, most recently from 5421c2a to 4b722bc Compare February 18, 2019 19:07
@nicolasstucki
Copy link
Contributor

Rebased

@odersky
Copy link
Contributor Author

odersky commented Feb 18, 2019

I accidentally pushed the wrong scalatest submodule in one of my commits. The last commit fixes that.

@odersky
Copy link
Contributor Author

odersky commented Feb 19, 2019

@nicolasstucki Why did you need to revert the scalatest community build? That does not work, as it now fails with an illegal comparison.

@nicolasstucki
Copy link
Contributor

I will re-revert it. I now noticed that my issue was due to a dirty build.

@odersky
Copy link
Contributor Author

odersky commented Feb 19, 2019

I'll do it as part of another push.

@odersky
Copy link
Contributor Author

odersky commented Feb 19, 2019

So we now have quoted idents as well: 'x works as a replacement for '{x}, but only inside spliced code. Outside, it is still rejected as a symbol literal.

@nicolasstucki
Copy link
Contributor

Finally, tests pass again

Quote(t)
}
else {
migrationWarningOrError(em"""symbol literal '${in.name} is no longer supported,
Copy link
Contributor

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?

Copy link
Contributor

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 '{}

@nicolasstucki
Copy link
Contributor

Looks good, but needs an extra reviewer as I added lots of commits. @biboudis will also review it.

@nicolasstucki
Copy link
Contributor

Rebased

@nicolasstucki
Copy link
Contributor

Rebased

Copy link
Contributor Author

@odersky odersky left a 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.

@odersky
Copy link
Contributor Author

odersky commented Feb 21, 2019

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
Copy link
Contributor

Choose a reason for hiding this comment

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

Super nitpick 🙈, TypSplice.

Copy link
Contributor

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]] }`.
Copy link
Contributor

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

@nicolasstucki nicolasstucki merged commit ac84182 into scala:master Feb 22, 2019
@ghost ghost removed the stat:needs review label Feb 22, 2019
@allanrenucci allanrenucci deleted the change-splice-dollar branch February 24, 2019 11:38
@biboudis biboudis added this to the 0.14 Tech Preview milestone Apr 5, 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.

None yet

3 participants