Allow syntax extensions which modify and decorate

[dylanmckay]

This adds a new SyntaxExtension type - Renovator.

A renovator is a syntax extension which can modify and decorate the items it is attached to - the union of MultiModifier and MultiDecorator.

This allows things such as syntax extensions which modify existing methods for types, and implement traits all in one.

This shouldn't break any existing code, and it only touches unstable compiler internals.

[rust-highfive]

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

[sfackler]

This has been something I've been meaning to do for a while - I think the whole split into decorators and modifiers was a bad design decision on my part. To that end, I'd like to move towards removing the other variants and making this the only one.

cc @nrc

[sfackler]

Seems like we can drop the return type and just have implementors push all of the things they're creating/preserving into push?

[dylanmckay]

That is a good idea, I'm adding this in now.

[bors]

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

[sfackler]

Looks like travis is failing and it needs a rebase. Looks good to me otherwise though.

r? @nrc

[nrc]

cc @cswords
Looks fine to me too. Note that we are looking at revamping syntax extensions completely over the next few months, so this is all likely to disappear. However, it's probably worth landing this since new syntax extensions will be closer to these Renovators than to the existing flavours.

[dylanmckay]

Travis seems to be having unrelated issues. apt-get update is failing.

[bors]

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

[dylanmckay]

Am working on fixing this up.

[dylanmckay]

@nrc: Have rewritten the PR so that renovators must push all items which they want to preserve.

This is ready for review.

[nrc]

Why must users push the items themselves? That seems a bit counter-intuitive. At the least it needs to be documented somewhere.

Given that the renovator behaviour is a strict super-set of decorator and modifier, it would be cool to see their code refactored to just use the renovator code. Would that be possible?

[dylanmckay]

Why must users push the items themselves?

@nrc: @sfackler suggested this behaviour in this comment. I agree with it, and it gives the most flexibility to extensions.

Happy to document it more.

Given that the renovator behaviour is a strict super-set of decorator and modifier, it would be cool to see their code refactored to just use the renovator code. Would that be possible?

It's tricky to merge the three, as there is special handling for the new things that modifiers create. I can merge the Decorator and Renovator expansion code however.

[bors]

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

[nrc]

@nrc: @sfackler suggested this behaviour in this comment. I agree with it, and it gives the most flexibility to extensions.

Happy to document it more.

Ok, that sounds good, more docs would be good though.

It's tricky to merge the three, as there is special handling for the new things that modifiers create. I can merge the Decorator and Renovator expansion code however.

What about the new things renovators create? Is this not the same handling?

[dylanmckay]

What about the new things renovators create? Is this not the same handling?

Modifiers have this special handling expand all the new stuff from the modifier. Decorators and renovators don't need it.

[nrc]

Why don't renovators need it, I'd have thought a renovator which doesn't create new items should behave exactly like a modifier?

[dylanmckay]

No idea, it's just what the current code does.

On Sat, Jun 11, 2016 at 2:51 AM, Nick Cameron notifications@github.com
wrote:

Why don't renovators need it, I'd have thought a renovator which doesn't
create new items should behave exactly like a modifier?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#33738 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AHXUrzYwCxP4zuHAVRaL3QwtKavuNMQxks5qKXoMgaJpZM4IiK3Y
.

[nrc]

This PR is superseded by #34253 which achieves a similar goal without adding a new class of macro. Sorry this PR didn't get to land and thanks for submitting it.