Skip to content

Inlining function defined in macro produces incorrect results (concatenated tokens). #12860

Closed
@zachs18

Description

@zachs18

Rust-analyzer's "Inline <function>" code action fails1 when the function to be inlined is defined in a macro.

macro_rules! define_function {
    () => {
        fn test_macro(x: bool) {
            if x { println!("x is true!"); }
        }
    };
}
define_function!();
fn test_free(x: bool) {
    if x { println!("x is true!"); }
}
fn main() {
    test_macro(true);
    test_free(true);
}

Result when selecting test_macro(true) and running the "Inline `test_macro`" action: (at least if and x should be separated)

// ...
fn main() {
    {
        let x = true; ifx{println!("x is true!");}};
    test_free(true);
}

Result when selecting test_free(true) and running the "Inline `test_free`" action: (to show that the issue only happens for functions defined in macros)

// ...
fn main() {
    test_macro(true);
    if true { println!("x is true!"); };
}

The problem also occurs in associated functions/methods. I originally ran into this error when trying to inline usize::next_multiple_of, which is defined in a macro.


rust-analyzer version: (eg. output of "Rust Analyzer: Show RA Version" command)
rust-analyzer version: 0.4.1138-standalone (977e12a 2022-07-23)
("Pre-Release" version on VSCode)
(Also happens on normal version on VSCode, rust-analyzer version: 0.3.1131-standalone (897a7ec 2022-07-17))

rustc version: (eg. output of rustc -V)
rustc 1.62.1 (e092d0b6b 2022-07-16)

relevant settings: (eg. client settings, or environment variables like CARGO, RUSTUP_HOME or CARGO_HOME)
None that I am aware are relevant.

Footnotes

  1. It succeeds but does not produce correct code if any sequential tokens need to be separated by spaces.

Activity

zachs18

zachs18 commented on Jul 24, 2022

@zachs18
ContributorAuthor

Additionally, when inlining methods defined in macros, self is not replaced.

struct Test;

macro_rules! define_method {
    () => {
        impl Test {
            fn test_method_macro(&self) {
                match self {
                    Test => println!("Test!")
                }
            }
        }
    };
}
define_method!();
fn main() {
    Test.test_method_macro();
}

becomes

// ...
fn main() {
    {
        let ref this = Test; matchself{Test=>println!("Test!")}};
}

when inlining Test.test_method_macro()

zachs18

zachs18 commented on Jul 25, 2022

@zachs18
ContributorAuthor

I inserted a dbg!(&body) at the beginning of fn inline on line 305 of crates/ide-assists/src/handlers/inline_call.rs, and the main difference between inlining fn test_free and fn test_macro appears to be that fn test_free contained WHITESPACE tokens, but fn test_macro did not.

My (limited, possibly wrong) understanding of what is happening (for the whitespace problem, no idea about self not being replaced1) is that rust-analyzer converts it's AST representation from SyntaxNodes (which contain comments and whitespace) to TokenTrees (which does not contain comments or whitespace)2 when expanding macros, so it may not be possible (without changes to how macros are expanded by rust-analyzer) to include the original whitespace&comments when inlining macro-produced functions.

Maybe it would be easier to just insert whitespace between tokens which cannot be adjacent? This is probably also required for inlining functions defined in proc-macros to work, since in that case there is no "original whitespace" that could even be kept.

Does rust-analyzer have a way to determine if a function was defined in a macro? rustc appears to have this (at least at some level), since if a function defined in a macro contains an error, it will say note: this error originates in the macro `define_func` (in Nightly builds, run with -Z macro-backtrace for more info)

Footnotes

  1. I also inserted dbg!(...) on the usages of fn inline on lines 127 and 221, which showed that the inlining of test_method (correctly) replaced self with this, but inlining test_method_macro did not. I have not looked further into why, but I think that and the whitespace problem may be separate issues

  2. I think this happens in syntax_node_to_token_tree_with_modifications on line 30 of crates/mbe/src/syntax_bridge.rs.

Veykril

Veykril commented on Jul 25, 2022

@Veykril
Member

Yes, macro expansions have no whitespace. This is a recurring thing and currently we just check for whether things come from a macro expansion in whicch ase we do whitespace insertion via https://github.com/rust-lang/rust-analyzer/blob/master/crates/ide-db/src/syntax_helpers/insert_whitespace_into_node.rs

added a commit that references this issue on Jul 26, 2022

Auto merge of #12877 - zachs18:inline-def-in-macro, r=zachs18

c2eebd7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions

      Inlining function defined in macro produces incorrect results (concatenated tokens). · Issue #12860 · rust-lang/rust-analyzer