Interpolate AST nodes in quasiquote. #23085

goffrie · 2015-03-05T20:08:39Z

This changes the ToTokens implementations for expressions, statements, etc. with almost-trivial ones that produce Interpolated(*Nt(...)) pseudo-tokens. In this way, quasiquote now works the same way as macros do: already-parsed AST fragments are used as-is, not reparsed.

The ToSource trait is removed. Quasiquote no longer involves pretty-printing at all, which removes the need for the encode_with_hygiene hack. All associated machinery is removed.

New Nonterminals are added: NtArm, NtImplItem, and NtTraitItem. These are just for quasiquote, not macros.

ToTokens is no longer implemented for Arg (although this could be added again) and Generics (which I don't think makes sense).

This breaks any compiler extensions that relied on the ability of ToTokens to turn AST fragments back into inspectable token trees. For this reason, this closes #16987.

As such, this is a [breaking-change].

Fixes #16472.
Fixes #15962.
Fixes #17397.
Fixes #16617.

rust-highfive · 2015-03-05T20:08:52Z

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @huonw (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. The way Github handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see CONTRIBUTING.md for more information.

kmcallister · 2015-03-07T03:00:30Z

cc me, @eddyb, @sfackler, @pnkfelix

eddyb · 2015-03-07T05:32:59Z

I have done this a long time ago in a branch I want to revive soon (the whole stage0 syntax extensions thing) - so I am pretty confident this is the way forward (I was too afraid of bootstrapping churn to split this specific change out).

goffrie · 2015-03-07T05:34:13Z

~~You're talking about the encode-idents-with-null-characters thing? AFAIK, I've ripped all that out - unless you happen to know of something I've missed?~~

eddyb · 2015-03-07T05:36:00Z

@goffrie ahhh you managed to see my comment before I realized I've missed that part in tbe description.

bors · 2015-03-09T23:27:41Z

☔ The latest upstream changes (presumably #22561) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2015-03-12T22:46:02Z

☔ The latest upstream changes (presumably #23265) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2015-03-20T09:48:04Z

☔ The latest upstream changes (presumably #23548) made this pull request unmergeable. Please resolve the merge conflicts.

lambda-fairy · 2015-03-30T08:07:12Z

Is anyone looking at this? I'm pretty excited about this PR.

kmcallister · 2015-03-30T15:15:39Z

Me too! It needs a rebase though.

goffrie · 2015-04-05T03:49:18Z

Sorry, I've been so busy 😭 I'll try to get this PR updated this week.

huonw · 2015-04-06T04:50:17Z

r? @kmcallister (transferring reviewership, don't have the bandwidth right now.)

goffrie · 2015-04-13T02:55:32Z

Rebased. Can someone review this?

huonw · 2015-04-15T22:35:22Z

Finally got time to review! This is very cool.

Just resolving the staging point one way or another, and I think this is good to go.

goffrie · 2015-04-19T02:04:59Z

@huonw I suspect that we would in fact encounter the problem you mentioned, so I removed the associated commit :( Also rebased. Should be good now?

huonw · 2015-04-21T14:54:05Z

That's a little sad: the quasiquoted version is definitely nicer.

@bors r+

bors · 2015-04-21T14:54:06Z

📌 Commit d61012d has been approved by huonw

bors · 2015-04-21T16:44:24Z

⌛ Testing commit d61012d with merge bfe5a6a...

bors · 2015-04-21T18:05:49Z

💔 Test failed - auto-linux-64-x-android-t

bors · 2015-04-23T19:40:39Z

☔ The latest upstream changes (presumably #24537) made this pull request unmergeable. Please resolve the merge conflicts.

goffrie · 2015-04-24T00:27:57Z

Rebased. All tests pass locally.

alexcrichton · 2015-04-24T00:36:58Z

@bors: r=huonw fb43942

bors · 2015-04-24T01:43:17Z

⌛ Testing commit fb43942 with merge 606c9d5...

bors · 2015-04-24T03:18:38Z

💔 Test failed - auto-mac-32-opt

lambda-fairy · 2015-04-24T03:52:16Z

The failure seems unrelated:

test [debuginfo-lldb] debuginfo/associated-types.rs ... 
command timed out: 1200 seconds without output, attempting to kill
process killed by signal 15
program finished with exit code -1
elapsedTime=2966.482249

This PR only touches libsyntax; it shouldn't have anything to do with debuginfo, right?

goffrie · 2015-04-24T03:55:24Z

Yeah, I don't think it's related.
On Apr 23, 2015 11:52 PM, "Chris Wong" notifications@github.com wrote:

The failure seems unrelated:

test [debuginfo-lldb] debuginfo/associated-types.rs ...
command timed out: 1200 seconds without output, attempting to kill
process killed by signal 15
program finished with exit code -1
elapsedTime=2966.482249

This PR only touches libsyntax; it shouldn't have anything to do with
debuginfo, right?

—
Reply to this email directly or view it on GitHub
#23085 (comment).

bors · 2015-04-24T08:44:24Z

☔ The latest upstream changes (presumably #24759) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2015-04-26T01:38:52Z

☔ The latest upstream changes (presumably #24718) made this pull request unmergeable. Please resolve the merge conflicts.

This changes the `ToTokens` implementations for expressions, statements, etc. with almost-trivial ones that produce `Interpolated(*Nt(...))` pseudo-tokens. In this way, quasiquote now works the same way as macros do: already-parsed AST fragments are used as-is, not reparsed. The `ToSource` trait is removed. Quasiquote no longer involves pretty-printing at all, which removes the need for the `encode_with_hygiene` hack. All associated machinery is removed. A new `Nonterminal` is added, NtArm, which the parser now interpolates. This is just for quasiquote, not macros (although it could be in the future). `ToTokens` is no longer implemented for `Arg` (although this could be added again) and `Generics` (which I don't think makes sense). This breaks any compiler extensions that relied on the ability of `ToTokens` to turn AST fragments back into inspectable token trees. For this reason, this closes rust-lang#16987. As such, this is a [breaking-change]. Fixes rust-lang#16472. Fixes rust-lang#15962. Fixes rust-lang#17397. Fixes rust-lang#16617.

Such things no longer exist.

tamird · 2015-04-26T02:24:06Z

src/test/run-pass-fulldeps/qquote.rs


-trait FakeExtCtxt {


why change all this?

I rewrote this test independently from your branch (from when it was ignore-test-ed and nonfunctional) and when rebasing I figured that my test already covered everything yours does, so I just took mine. (Also, it seems kind of fragile to me IMO to fake the ExtCtxt methods that quasiquote happens to need, instead of creating an actual ExtCtxt.)

Right, so I agree with that part, but if you're going to change that, there are 2 other places where a FakeExtCtxt is defined and used, and it would be good to fix them as well. As for this file's main function, there's really no need to change it; it's just noise :(

Um, I changed the main function for a reason - I'm testing unquotes now (which is what this branch is all about). I'll change the other files that use a FakeExtCtxt, though.

I'm not sure what you mean, but it looks to me like the main function is effectively unchanged, just rearranged.

I'm testing that not only (e.g.) quote_expr!(cx, 23) works and produces 23, but also that quote_expr!(cx, $expr) (where expr is the result of the earlier quote) still produces 23.

Instead create an ExtCtxt structure.

pnkfelix · 2015-04-26T09:52:27Z

@bors: r=huonw 24ef905

bors · 2015-04-26T09:52:29Z

⌛ Testing commit 24ef905 with merge 6365080...

This changes the `ToTokens` implementations for expressions, statements, etc. with almost-trivial ones that produce `Interpolated(*Nt(...))` pseudo-tokens. In this way, quasiquote now works the same way as macros do: already-parsed AST fragments are used as-is, not reparsed. The `ToSource` trait is removed. Quasiquote no longer involves pretty-printing at all, which removes the need for the `encode_with_hygiene` hack. All associated machinery is removed. New `Nonterminal`s are added: NtArm, NtImplItem, and NtTraitItem. These are just for quasiquote, not macros. `ToTokens` is no longer implemented for `Arg` (although this could be added again) and `Generics` (which I don't think makes sense). This breaks any compiler extensions that relied on the ability of `ToTokens` to turn AST fragments back into inspectable token trees. For this reason, this closes #16987. As such, this is a [breaking-change]. Fixes #16472. Fixes #15962. Fixes #17397. Fixes #16617.

bors · 2015-04-26T12:43:48Z

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-32-nopt-t, auto-win-32-opt, auto-win-64-nopt-t, auto-win-64-opt

kmcallister · 2015-04-26T19:25:54Z

🎉

See rust-lang/rust#23085 Close #18

rust syntax extension changes: rust-lang/rust#23085 reference fix: tomjakubowski/json_macros@d4d3133

rust-highfive assigned huonw Mar 5, 2015

goffrie force-pushed the interpolating-quote branch from e1a2420 to 39c9470 Compare March 10, 2015 04:17

goffrie force-pushed the interpolating-quote branch 2 times, most recently from c4933fc to 5173007 Compare March 16, 2015 04:49

rust-highfive assigned kmcallister and unassigned huonw Apr 6, 2015

kmcallister removed their assignment Apr 9, 2015

goffrie force-pushed the interpolating-quote branch from 5173007 to 6025cd2 Compare April 9, 2015 21:31

huonw self-assigned this Apr 15, 2015

goffrie force-pushed the interpolating-quote branch from 6025cd2 to d61012d Compare April 19, 2015 02:03

goffrie force-pushed the interpolating-quote branch from dce933b to fb43942 Compare April 23, 2015 20:55

goffrie force-pushed the interpolating-quote branch from fb43942 to 0dbee13 Compare April 24, 2015 15:31

goffrie added 2 commits April 25, 2015 21:42

Remove remaining tests for hygiene-encoded identifiers.

ea892dc

Such things no longer exist.

goffrie force-pushed the interpolating-quote branch from 0dbee13 to ea892dc Compare April 26, 2015 01:42

tamird reviewed Apr 26, 2015
View reviewed changes

Remove FakeExtCtxt from qquote tests.

24ef905

Instead create an ExtCtxt structure.

bors merged commit 24ef905 into rust-lang:master Apr 26, 2015

This was referenced Apr 26, 2015

syntax: Repeating fragments in quasiquotation #24224

Closed

Add rfail-full test group for qquote.rs #24835

Merged

ptal mentioned this pull request Apr 28, 2015

Unexpected interpolate tokens in quote_item! macro #24914

Closed

tomjakubowski mentioned this pull request May 4, 2015

macros fail to compile with latest Rust nightly build tomjakubowski/json_macros#18

Closed

tomjakubowski added a commit to tomjakubowski/json_macros that referenced this pull request May 4, 2015

Update for interpolation changes upstream

d4d3133

See rust-lang/rust#23085 Close #18

sunng87 added a commit to sunng87/tojson_macros that referenced this pull request May 9, 2015

(fix) keep compatible with upstream changes.

39a51e3

rust syntax extension changes: rust-lang/rust#23085 reference fix: tomjakubowski/json_macros@d4d3133

goffrie deleted the interpolating-quote branch October 12, 2017 04:24

Interpolate AST nodes in quasiquote. #23085

Interpolate AST nodes in quasiquote. #23085

Conversation

goffrie commented Mar 5, 2015

rust-highfive commented Mar 5, 2015

kmcallister commented Mar 7, 2015

eddyb commented Mar 7, 2015

goffrie commented Mar 7, 2015

eddyb commented Mar 7, 2015

bors commented Mar 9, 2015

bors commented Mar 12, 2015

bors commented Mar 20, 2015

lambda-fairy commented Mar 30, 2015

kmcallister commented Mar 30, 2015

goffrie commented Apr 5, 2015

huonw commented Apr 6, 2015

goffrie commented Apr 13, 2015

huonw commented Apr 15, 2015

goffrie commented Apr 19, 2015

huonw commented Apr 21, 2015

bors commented Apr 21, 2015

bors commented Apr 21, 2015

bors commented Apr 21, 2015

bors commented Apr 23, 2015

goffrie commented Apr 24, 2015

alexcrichton commented Apr 24, 2015

bors commented Apr 24, 2015

bors commented Apr 24, 2015

lambda-fairy commented Apr 24, 2015

goffrie commented Apr 24, 2015

bors commented Apr 24, 2015

bors commented Apr 26, 2015

tamird Apr 26, 2015

Choose a reason for hiding this comment

goffrie Apr 26, 2015

Choose a reason for hiding this comment

tamird Apr 26, 2015

Choose a reason for hiding this comment

goffrie Apr 26, 2015

Choose a reason for hiding this comment

tamird Apr 26, 2015

Choose a reason for hiding this comment

goffrie Apr 26, 2015

Choose a reason for hiding this comment

pnkfelix commented Apr 26, 2015

bors commented Apr 26, 2015

bors commented Apr 26, 2015

kmcallister commented Apr 26, 2015