Compiler loses location information before calling macros (sometimes) #43081

alexcrichton · 2017-07-06T01:39:27Z

Updated description

First implemented in #43230 the compiler can now tokenize a few AST nodes when necessary losslessly from the original token stream. Currently, however, this is somewhat buggy:

It only happens for items in certain situations (aka no inner attributes). The consequence of this is that more and more attributed items will have invalid span information when sent to procedural macros.
We don't invalidate the cache when later changing the item. The consequence of this is that procedural macros will receive a buggy view of what the AST node actually represents.

The "real bug" here is that we haven't actually implemented converting an AST to a token tree. The initial implementation in #43230 was effectively just a heuristic, and not even a great one! As a result we still need the ability basically to losslessly tokenize an AST node back to its original set of tokens.

Some bugs that arise from this are:

Usage of #[cfg] modifies the AST but doesn't invalidate the cache -- Macros 2.0: #[cfg_attr] makes .to_string() and TokenStream disagree #48644
Modules with attributes disagree on what their internal tokens are -- Inconsistency between Display and IntoIterator for a TokenStream containing a module #47627
macro_rules! and procedural macros don't play well together -- Inconsistent tokenization of procedural macros with macro_rules #49846
~~Invoking a macro with brackets loses span information -- Procedural macros stringify inputs with macro invocations too aggressively #50840~~

Original Description

There's an associated FIXME in the code right now, and to fix this we'll need to implement tokenization of an AST node. Right now the thinking of how to implement this is to save all TokenStream instances adjacent to an AST node, and use that instead of converting back into a token stream

cc @dtolnay, @nrc, @jseyfried

The text was updated successfully, but these errors were encountered:

This partly resolves the `FIXME` located in `src/libproc_macro/lib.rs` when interpreting interpolated tokens. All instances of `ast::Item` which have a list of tokens attached to them now use that list of tokens to losslessly get converted into a `TokenTree` instead of going through stringification and losing span information. cc rust-lang#43081

Implement tokenization for some items in proc_macro This PR is a partial implementation of #43081 targeted towards preserving span information in attribute-like procedural macros. Currently all attribute-like macros will lose span information with the input token stream if it's iterated over due to the inability of the compiler to losslessly tokenize an AST node. This PR takes a strategy of saving off a list of tokens in particular AST nodes to return a lossless tokenized version. There's a few limitations with this PR, however, so the old fallback remains in place.

This commit updates the `Mac_` AST structure to keep track of the delimiters that it originally had for its invocation. This allows us to faithfully pretty-print macro invocations not using parentheses (e.g. `vec![...]`). This in turn helps procedural macros due to rust-lang#43081. Closes rust-lang#50840

rustc: Correctly pretty-print macro delimiters This commit updates the `Mac_` AST structure to keep track of the delimiters that it originally had for its invocation. This allows us to faithfully pretty-print macro invocations not using parentheses (e.g. `vec![...]`). This in turn helps procedural macros due to #43081. Closes #50840

…nd, r=petrochenkov Fix recursive nonterminal expansion during pretty-print/reparse check Makes progress towards rust-lang#43081 In PR rust-lang#73084, we started recursively expanded nonterminals during the pretty-print/reparse check, allowing them to be properly compared against the reparsed tokenstream. Unfortunately, the recursive logic in that PR only handles the case where a nonterminal appears inside a `TokenTree::Delimited`. If a nonterminal appears directly in the expanded tokens of another nonterminal, the inner nonterminal will not be expanded. This PR fixes the recursive expansion of nonterminals, ensuring that they are expanded wherever they occur.

Fixes rust-lang#74616 Makes progress towards rust-lang#43081 Unblocks PR rust-lang#76130 When pretty-printing an AST node, we may insert additional parenthesis to ensure that precedence is properly preserved in code we output. However, the proc macro implementation relies on comparing a pretty-printed AST node to the captured `TokenStream`. Inserting extra parenthesis changes the structure of the reparsed `TokenStream`, making the comparison fail. This PR refactors the AST pretty-printing code to allow skipping the insertion of additional parenthesis. Several freestanding methods are moved to trait methods on `PrintState`, which keep track of an internal `insert_extra_parens` flag. This flag is normally `true`, but we expose a public method which allows pretty-printing a nonterminal with `insert_extra_parens = false`. To avoid changing the public interface of `rustc_ast_pretty`, the freestanding `_to_string` methods are changed to delegate to a newly-crated `State`. The main pretty-printing code is moved to a new `state` module to ensure that it does not accidentally call any of these public helper functions (instead, the internal functions with the same name should be used).

…ochenkov Refactor AST pretty-printing to allow skipping insertion of extra parens Fixes rust-lang#75734 Makes progress towards rust-lang#43081 Unblocks PR rust-lang#76130 When pretty-printing an AST node, we may insert additional parenthesis to ensure that precedence is properly preserved in code we output. However, the proc macro implementation relies on comparing a pretty-printed AST node to the captured `TokenStream`. Inserting extra parenthesis changes the structure of the reparsed `TokenStream`, making the comparison fail. This PR refactors the AST pretty-printing code to allow skipping the insertion of additional parenthesis. Several freestanding methods are moved to trait methods on `PrintState`, which keep track of an internal `insert_extra_parens` flag. This flag is normally `true`, but we expose a public method which allows pretty-printing a nonterminal with `insert_extra_parens = false`. To avoid changing the public interface of `rustc_ast_pretty`, the freestanding `_to_string` methods are changed to delegate to a newly-crated `State`. The main pretty-printing code is moved to a new `state` module to ensure that it does not accidentally call any of these public helper functions (instead, the internal functions with the same name should be used).

gui1117 · 2020-11-13T16:30:51Z

some question on this issue:

Is there some in-progress work or does some people thought about what is needed in order to fix the fact that usage of #[cfg(features="some_not_set")] loses the spans ? I would like to provide some help (if it's in my skill/time)
do we have a more complete list of all situation losing the span in proc_macros ? I am aware of:
- outer attribute, with WIP PR [WIP] Token-based outer attributes handling #76130
- usage of cfg stuff

Thanks all

Aaron1011 · 2020-11-13T17:01:07Z

@thiolliere: Summary of the current progress:

~~PR Parse bang macro as a statement when used in trailing expr position #78991 fixes an inconsistency in parsing of statement macros (it's currently waiting on a Crater run)~~ EDIT: This will cause a significant amount of breakage, and will take time to roll out. Fortunately, it doesn't block progress on this issue

PR Properly handle attributes on statements #78296 fixes our handling of attributes on statements
Once that PR is merged, I will continue to split up [WIP] Token-based outer attributes handling #76130

Once all of the pieces of #76130 are merged, any code that compiles on stable should have all spans available (except for any additional pretty-printer issues that may get discovered).

Using proc-macros as inner attributes will still cause spans to be lost, but such usage is nightly-only.

…k, r=petrochenkov Replace pretty-print/compare/retokenize hack with targeted workarounds Based on rust-lang#78296 cc rust-lang#43081 The 'pretty-print/compare/retokenize' hack is used to try to avoid passing an outdated `TokenStream` to 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 unhygienic `macro_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 rust-lang#79338 This PR removes the pretty-print/compare/retokenize logic from `nt_to_tokenstream`. We only discard the original `TokenStream` under two circumstances: * Inner attributes are used (detected by examining the AST) * `cfg`/`cfg_attr` processing 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 original `TokenStream`. In all other cases, we preserve the original `TokenStream`. This could use a bit of refactoring/renaming - opening for a Crater run. r? `@ghost`

Aaron1011 · 2021-01-19T01:26:47Z

Progress update: The pretty-print/retokenize check was removed in #79472. We now only lose spans when we explicitly remove the original TokenStream. This happens in two cases:

The input to a derive proc-macro has cfg or cfg_attr attributes (except when a #[cfg] attribute is kept due to its predicate evaluating to true)
Inner attributes are present

PR #80689 completely resolves this issue. However, it is quite large, so I'm in the process of splitting it into independent pieces for easier review.

Aaron1011 · 2021-04-11T11:21:16Z

PR #82608 has been merged! We now preserve spans for all stable uses of macros (e.g. custom derives), and nearly all unstable usages (e.g. inner attrs).

If anyone encounters a loss of location information, please open a new issue for the specific problem you're having.

537: Add workaround for a rustc bug (rust/43081) r=halzy a=toasteater This works around a compiler bug which can sometimes cause E0425 when the `#[methods]` macro is used. The error, when triggered, would show up as `error[E0425]: cannot find value `builder` in this scope` without any span information. This is probably an instance of rust-lang/rust#43081 Co-authored-by: toasteater <[email protected]>

alexcrichton added the A-macros-2.0 Area: Declarative macros 2.0 (#39412) label Jul 6, 2017

This was referenced Jul 6, 2017

Work on nightly, not a fork alexcrichton/futures-await#1

Closed

How to parse keeping spans dtolnay/syn#178

Closed

mystor mentioned this issue Jul 7, 2017

Release 0.12 dtolnay/syn#123

Closed

alexcrichton mentioned this issue Jul 14, 2017

Implement tokenization for some items in proc_macro #43230

Merged

Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 28, 2017

antoyo mentioned this issue Jul 31, 2017

Better error reporting antoyo/relm#27

Closed

alexcrichton mentioned this issue Mar 2, 2018

Macros 2.0: #[cfg_attr] makes .to_string() and TokenStream disagree #48644

Closed

alexcrichton mentioned this issue Apr 2, 2018

Inconsistent tokens between the same struct defined inside vs outside of function #49604

Closed

This was referenced Apr 10, 2018

Inconsistent tokenization of procedural macros with macro_rules #49846

Closed

TypeError: wasm.__wbindgen_global_argument_ptr is not a function rustwasm/wasm-bindgen#113

Closed

proc_macro: Avoid cached TokenStream more often #49852

Merged

alexcrichton mentioned this issue Apr 19, 2018

Hygiene break in macros involving string containing single quote #50061

Closed

alexcrichton mentioned this issue Apr 27, 2018

Attribute macros invoked at crate root have issues #41430

Open

alexcrichton mentioned this issue May 17, 2018

Procedural macros stringify inputs with macro invocations too aggressively #50840

Closed

alexcrichton mentioned this issue May 22, 2018

rustc: Correctly pretty-print macro delimiters #50971

Merged

alexcrichton added the A-macros-1.2 Area: Declarative macros 1.2 label May 22, 2018

This was referenced May 22, 2018

Attributes used with derive have incorrect span information #47941

Closed

Derive for items defined inside a function receive incorrect span information #47983

Closed

alexcrichton added C-bug Category: This is a bug. and removed C-feature-request Category: A feature request, i.e: not implemented / a PR. labels May 22, 2018

Aaron1011 mentioned this issue Oct 8, 2020

Unexpected Regression in Rust 1.47 for Custom Derives #77718

Closed

Aaron1011 mentioned this issue Oct 10, 2020

Build failing after update to Rust 1.47.0 #77789

Closed

Aaron1011 mentioned this issue Oct 11, 2020

Remove compiler-synthesized reexports when documenting #77785

Merged

emmanueltouzery mentioned this issue Oct 24, 2020

Bad spans from type error within code expanded from proc macro attribute #71968

Closed

jebrosen mentioned this issue Oct 30, 2020

Error location of compilation error is missing if code is inside a rocket route handler rwf2/Rocket#1456

Closed

gui1117 mentioned this issue Nov 10, 2020

Add pallet attribute macro to declare pallets paritytech/substrate#6877

Merged

3 tasks

Aaron1011 mentioned this issue Nov 27, 2020

Replace pretty-print/compare/retokenize hack with targeted workarounds #79472

Merged

petrochenkov mentioned this issue Jan 19, 2021

Remove hack for actix-web and actori-web #81174

Closed

Aaron1011 closed this as completed Apr 11, 2021

dtolnay mentioned this issue Oct 25, 2021

Rustc passes syntactically invalid input to attribute macros #90256

Open

dtolnay mentioned this issue Dec 4, 2021

Allow using $:literal containing integer to index into a tuple #91166

Closed

mystor mentioned this issue Jun 19, 2022

Consider removing invalid span workaround form join_spans dtolnay/quote#222

Closed

dtolnay mentioned this issue Oct 25, 2022

Remove 'rustc is so bad at spans' workaround dtolnay/remain#19

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Compiler loses location information before calling macros (sometimes) #43081

Compiler loses location information before calling macros (sometimes) #43081

alexcrichton commented Jul 6, 2017 •

edited by dtolnay

Loading

gui1117 commented Nov 13, 2020 •

edited

Loading

Aaron1011 commented Nov 13, 2020 •

edited

Loading

Aaron1011 commented Jan 19, 2021

Aaron1011 commented Apr 11, 2021 •

edited

Loading

Compiler loses location information before calling macros (sometimes) #43081

Compiler loses location information before calling macros (sometimes) #43081

Comments

alexcrichton commented Jul 6, 2017 • edited by dtolnay Loading

Updated description

Original Description

gui1117 commented Nov 13, 2020 • edited Loading

Aaron1011 commented Nov 13, 2020 • edited Loading

Aaron1011 commented Jan 19, 2021

Aaron1011 commented Apr 11, 2021 • edited Loading

alexcrichton commented Jul 6, 2017 •

edited by dtolnay

Loading

gui1117 commented Nov 13, 2020 •

edited

Loading

Aaron1011 commented Nov 13, 2020 •

edited

Loading

Aaron1011 commented Apr 11, 2021 •

edited

Loading