Replace pretty-print/compare/retokenize hack with targeted workarounds#79472
Merged
bors merged 1 commit intorust-lang:masterfrom Dec 30, 2020
Merged
Replace pretty-print/compare/retokenize hack with targeted workarounds#79472bors merged 1 commit intorust-lang:masterfrom
bors merged 1 commit intorust-lang:masterfrom
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on #78296
cc #43081
The 'pretty-print/compare/retokenize' hack is used to try to avoid passing an outdated
TokenStreamto a proc-macro when the underlying AST is modified in some way (e.g. cfg-stripping before derives). Unfortunately, retokenizing throws away spans (including hygiene information), which causes issues of its own. Every improvement to the accuracy of the pretty-print/retokenize comparison has resulted in non-trivial ecosystem breakage due to hygiene changes. In extreme cases, users deliberately wrote unhygienicmacro_rules!macros (likely because they did not realize that the compiler's behavior was a bug).Additionaly, the comparison between the original and pretty-printed/retoknized token streams comes at a non-trivial runtime cost, as shown by #79338
This PR removes the pretty-print/compare/retokenize logic from
nt_to_tokenstream. We only discard the originalTokenStreamunder two circumstances:cfg/cfg_attrprocessing modifies the AST. This is detected by making the visitor update a flag when it performs a modification, instead of trying to detect the modification after-the-fact. Note that a 'matching'cfg(e.g.#[cfg(not(FALSE)]) does not actually get removed from the AST, allowing us to preserve the originalTokenStream.In all other cases, we preserve the original
TokenStream.This could use a bit of refactoring/renaming - opening for a Crater run.
r? @ghost