Macro resolution regression when a #[test] is named 'panic'

[SimonSapin]

Test case:

#[test]
fn panic() {
    assert!(true)
}

$ rustc +nightly -V
rustc 1.30.0-nightly (63d66494a 2018-08-23)
$ cargo +nightly test
error: cannot determine resolution for the macro `panic`
 --> src/lib.rs:3:5
  |
3 |     assert!(true)
  |     ^^^^^^^^^^^^^
  |
  = note: import resolution is stuck, try simplifying macro imports

error: aborting due to previous error

error: Could not compile `foo`.

CC @petrochenkov

[SimonSapin]

When removing the assert (to get past the error), the output of --pretty=expanded is:

#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use ::std::prelude::v1::*;
#[macro_use]
extern crate std;
#[test]
pub fn panic() {

}
#[allow(unused)]
use self::panic as panic;
pub mod __test_reexports {
    pub use super::panic;
}
pub mod __test {
    extern crate test;
    #[main]
    pub fn main() -> () { test::test_main_static(TESTS) }
    const TESTS: &'static [self::test::TestDescAndFn] =
        &[self::test::TestDescAndFn{desc:
                                        self::test::TestDesc{name:
                                                                 self::test::StaticTestName("panic"),
                                                             ignore: false,
                                                             should_panic:
                                                                 self::test::ShouldPanic::No,
                                                             allow_fail:
                                                                 false,},
                                    testfn:
                                        self::test::StaticTestFn(||
                                                                     self::test::assert_test_result(::__test_reexports::panic())),}];
}

Maybe it’s the use self::panic as panic; item that gets macro resolution confused? I’d expect that it only imports a function, not a macro, and they should be in separate namespaces.

[petrochenkov]

Something is wrong with path resolution for macros in expression position specifically.

If you add a semicolon everything compiles.

#[test]
fn panic() {
    assert!(true); // OK with semicolon
}

[SimonSapin]

FWIW in the non-reduced file https://github.com/servo/rust-mozjs/blob/dd338dae9bb68fa0cf23d4da21afc10e0153b27c/src/panic.rs the error is reported twice, one of them on a statement-macro with a semicolon:

error: cannot determine resolution for the macro `panic`
  --> tests/panic.rs:46:9
   |
46 |         panic!()
   |         ^^^^^
   |
   = note: import resolution is stuck, try simplifying macro imports

error: cannot determine resolution for the macro `panic`
  --> tests/panic.rs:37:9
   |
37 |         assert!(!function.is_null());
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   |
   = note: import resolution is stuck, try simplifying macro imports

error: aborting due to 2 previous errors

[petrochenkov]

IIRC, the second case is reported only because the first error (without semicolon) breaks everything.

This issue is kinda known, see proc-macro-attributes.rs in 674a5db.

[pnkfelix]

visited for triage. After some discussion about severity, decided to tag as P-high.

[nikomatsakis]

@petrochenkov what is required to fix this bug? Do we need to land something like #53913?

[petrochenkov]

@nikomatsakis
I don't know, I didn't investigate what happens in this case, but #53913 is unlikely to be related.

[nikomatsakis]

@petrochenkov this seems kind of important to figure out, no? Have you had any further thoughts? If not, I'd be up to try and investigate a bit and see if I can figure something out.

[pnkfelix]

visited for triage. Curious about an update from @petrochenkov about how to approach this and whether they have time to attack it. Assigning to self for further investigation/management.

[petrochenkov]

Status update: still plan to investigate this (probably this week).
The delay is mostly due to prioritization - there were other regressions and urgent future-proofing work to complete before beta, but now all of that is done.

[petrochenkov]

Not sure how exactly, but this is already fixed.
#54209 adds tests.