Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Errnous "unexpected token in input" #8828

Open
leehambley opened this issue May 13, 2021 · 9 comments · Fixed by #14781
Open

Errnous "unexpected token in input" #8828

leehambley opened this issue May 13, 2021 · 9 comments · Fixed by #14781
Labels
A-macro macro expansion A-nameres name, path and module resolution E-hard S-actionable Someone could pick this issue up and work on it right now

Comments

@leehambley
Copy link

rust-analyzer version: fd109fb58 2021-05-10 stable

Given the following piece of test code:

    macro_rules! test_driver {
        ($driver_name:ident, $setup:expr) => {
            concat_idents!(
                test_name = "test_",
                $driver_name,
                "_driver_raises_malformed_error_when_migration_has_both_change_and_up_steps",
                {
                    #[test]
                    fn test_name() -> Result<(), String> {
                        let mut driver = $setup;
                        match driver.apply(&malformed_migration()) {
                            Ok(_) => Err(format!("engine did not report malformed error",)),
                            Err(e) => match e {
                                Error::MalformedMigration => Ok(()),
                                _ => Err(format!("Engine returned {:?}, unexpected", e)),
                            },
                        }?;
                        Ok(())
                    }
                }
            );
        };
    }

    // rust-analyzer bug here (?)
    // https://github.com/rust-analyzer/rust-analyzer/issues/6747
    test_driver!(noop_driver, { NoopDriver {} });
    test_driver!(succeed_or_fail_driver, { SucceedOrFailDriver {} });

image

This is the first time I wrote a macro, so very possible that I made a mistake, but I don't think so, as the example is more-or-less verbatim from the concat_idents crate docs.

I tried restarting the server, and confirmed that after a few seconds the error returns.

Googling led me to #6747, I don't see any other issues open now with "unexpected token in input" error.

I tried to reproduce minimally, but I cannot reproduce it. My best guess is that because Driver in my original example is a trait, something different happens?

Here is the repro case with simple empty structs, showing a clean vscode build, and the crate version for concat_idents

image

leehambley added a commit to leehambley/mitre that referenced this issue May 13, 2021
Probably this is the wrong way to go, but has been useful experience.

Opened an issue with rust-analyzer here:

- rust-lang/rust-analyzer#8828

Something weird (and not reproducible) with macros happening.
@Veykril Veykril added A-macro macro expansion S-actionable Someone could pick this issue up and work on it right now labels May 13, 2021
@edwin0cheng edwin0cheng added the A-nameres name, path and module resolution label May 13, 2021
@edwin0cheng
Copy link
Member

edwin0cheng commented May 13, 2021

MWE:

use concat_idents::concat_idents;

fn main() {
    test_b_test_test();
}

concat_idents!(
    test_name = "test_",
    b,
    "_test_test",
    {
        fn test_name() {}
    }
);  // <- ERROR: unexpected token in input

However, the following code works :

use concat_idents::concat_idents as cat;

fn main() {
    test_b_test_test();
}

cat!(
    test_name = "test_",
    b,
    "_test_test",
    {
        fn test_name() {}
    }
);

So It seem like that we incorrectly resolve the name of concat_idents to builtin one.

bors bot added a commit that referenced this issue May 13, 2021
8830: feat: Implement bulitin macro `concat_idents` r=edwin0cheng a=edwin0cheng

cc  #8828

bors r+

Co-authored-by: Edwin Cheng <edwin0cheng@gmail.com>
@edwin0cheng
Copy link
Member

After #8830, the resolution is still incorrect :

concat_idents

@Veykril
Copy link
Member

Veykril commented May 13, 2021

@jonas-schievink
Copy link
Contributor

I think we consider the prelude to be always #[macro_use]'d

This part seems correct, the crate whose prelude we import gets an implicit #[macro_use], so that stuff like vec![] is in scope.

Even though these are in legacy macro scope, they can apparently be shadowed by non-legacy macros, which doesn't really match what we do, so I'd say path resolution is to blame. We probably need to look into what rustc does here.

@jonas-schievink jonas-schievink self-assigned this Jul 1, 2021
@jonas-schievink
Copy link
Contributor

Hmm, it seems like one issue here is that we eagerly try to resolve and expand legacy macros here: https://github.com/rust-analyzer/rust-analyzer/blob/e8b9f43084d6bfa3a77d50dd7759799d7aaf432c/crates/hir_def/src/nameres/collector.rs#L1892

With proper handling of the macro-use-prelude in place, the macro invocation will resolve to the prelude macro there and get expanded accordingly, but that's wrong, since the use shadows it!

If we don't allow that code path to resolve to macro-use-prelude macros, then the macro's path is rewritten to start with self:::

https://github.com/rust-analyzer/rust-analyzer/blob/e8b9f43084d6bfa3a77d50dd7759799d7aaf432c/crates/hir_def/src/nameres/collector.rs#L1940-L1944

...which then means that it'll never resolve to a prelude macro.

We do this rewriting to ensure that resolution during the fixed-point loop does not try to find legacy-scoped macros (since they are lexically scoped, you shouldn't be able to use one if it is defined later in the file).

This might be an indication that we need to model macro invocations and macro_rules! items as adding an entirely new scope in which new legacy macros may be introduced. This is what rustc does.

@jplatte
Copy link
Contributor

jplatte commented Apr 21, 2023

I get this error a lot recently for simple matches! invocations:

Screenshot_2023-04-21_111521

@Veykril
Copy link
Member

Veykril commented Apr 21, 2023

thats #9055, the matches macro was changed to use that now

@lowr
Copy link
Contributor

lowr commented May 15, 2023

This is unfortunately "unfixed" by #14800. As mentioned by @jonas-schievink, I'm inclined to believe we need to "model macro invocations and macro_rules! items as adding an entirely new scope in which new legacy macros may be introduced" instead of our current strategy: immediately expand macros in mod item collection.

Note that other issues aren't affected by #14800. It only affects function-like macro invocations at the top level of each module.

@lowr lowr reopened this May 15, 2023
@xTachyon
Copy link

Something very similar happened to me as well, with the same crate mentioned in the first comment.

The workaround was simple enough, just fully qualify the macro, but it took some time to figure the problem. When I initially wrote the code last year I remember it working fine, but now r-a errors on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-macro macro expansion A-nameres name, path and module resolution E-hard S-actionable Someone could pick this issue up and work on it right now
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants