Skip to content

Renumber AST nodes after expansion #9040

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Conversation

nikomatsakis
Copy link
Contributor

After expansion, fold and renumber AST nodes to ensure that each AST node has a
unique id. Fixes numerous bugs in macro expansion and deriving. Add two
representative tests.

Fixes #7971
Fixes #6304
Fixes #8367
Fixes #8754
Fixes #8852
Fixes #7654

r? @brson

…node has a

unique id. Fixes numerous bugs in macro expansion and deriving. Add two
representative tests.

Fixes rust-lang#7971
Fixes rust-lang#6304
Fixes rust-lang#8367
Fixes rust-lang#8754
Fixes rust-lang#8852
@huonw
Copy link
Member

huonw commented Sep 7, 2013

Needs a rebase. (Also, looks like it fixes #7654; editing this into the PR message so that bors will close that issue automatically.)

fn fold_view_path_(@self, vp: &view_path_) -> view_path_ {
match *vp {
view_path_simple(ident, ref path, node_id) => {
view_path_simple(ident,
Copy link
Member

Choose a reason for hiding this comment

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

Should this call self.fold_ident(ident)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should this call self.fold_ident(ident)?

Yes, good catch.

@nikomatsakis
Copy link
Contributor Author

never mind, I remember there was one bug I wanted to fix first

Jarcho pushed a commit to Jarcho/rust that referenced this pull request Aug 29, 2022
…=dswij

Add new lint [`positional_named_format_parameters`]

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: Add new lint [`positional_named_format_parameters`] to warn when named parameters in format strings are used as positional arguments.
Jarcho pushed a commit to Jarcho/rust that referenced this pull request Aug 29, 2022
Refactor `FormatArgsExpn`

It now for each format argument `{..}` has:
- The `Expr` it points to, and how it does so (named/named inline/numbered/implicit)
- The parsed `FormatSpec` (format trait/fill/align/etc., the precision/width and any value they point to)
- Many spans

The caller no longer needs to pair up arguments to their value, or separately interpret the `specs` `Expr`s when it isn't `None`

The gist is that it combines the result of [`rustc_parse_format::Parser`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_parse_format/struct.Parser.html) with the macro expansion itself

This unfortunately makes the code a bit longer, however we need to use both as neither have all the information we're after. `rustc_parse_format` doesn't have the information to resolve named arguments to their values. The macro expansion doesn't contain whether the positions are implicit/numbered/named, or the spans for format arguments

Wanted by rust-lang#9233 and rust-lang#8518 to be able to port the changes from rust-lang#9040

Also fixes rust-lang#8643, previously the format args seem to have been paired up with the wrong values somehow

changelog: [`format_in_format_args`]: Fix false positive due to misattributed arguments

r? `@flip1995`
cc `@nyurik`
# for free to join this conversation on GitHub. Already have an account? # to comment