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

@petrochenkov

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

Work around Nightly bug

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.

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
}

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

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.

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

@petrochenkov

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

@nikomatsakis

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

@petrochenkov

@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.

@petrochenkov

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.

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.

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

Rollup merge of rust-lang#54209 - petrochenkov:mexpr, r=pnkfelix

Rollup merge of rust-lang#54225 - pnkfelix:issue-53675-add-test-calle…

Auto merge of #54319 - GuillaumeGomez:rollup, r=GuillaumeGomez

Rollup merge of rust-lang#81387 - c410-f3r:tests-tests-tests, r=petro…

Navigation Menu

Repository navigation

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

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

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

Description

Activity

SimonSapin commented on Aug 24, 2018

petrochenkov commented on Aug 24, 2018

SimonSapin commented on Aug 24, 2018

petrochenkov commented on Aug 24, 2018

pnkfelix commented on Aug 30, 2018

nikomatsakis commented on Sep 6, 2018

petrochenkov commented on Sep 6, 2018

nikomatsakis commented on Sep 13, 2018

pnkfelix commented on Sep 13, 2018

petrochenkov commented on Sep 13, 2018

petrochenkov commented on Sep 13, 2018

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions