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

Regression: : cannot determine resolution for the attribute macro `test` #56375

Open
rustonaut opened this Issue Nov 30, 2018 · 9 comments

Comments

Projects
None yet
6 participants
@rustonaut
Copy link

rustonaut commented Nov 30, 2018

We have a regression in our internal rust backend between v1.29.0 and v1.30.0.

The build stops compiling when run with cargo test. But builds fine when run with
cargo +1.29.0 test, Also cargo build and cargo test --lib do work.

The error is:

error: cannot determine resolution for the attribute macro `test`
   --> tests/integration_tests/main.rs:141:3
    |
141 | #[test]
    |   ^^^^
    |
    = note: import resolution is stuck, try simplifying macro imports

As it's an internal repository I can't link it here but I will see if I can create
a minimal example producing this error.

It should be noted that the integration tests do

  1. not define any new macro
  2. only import macros via. #[macro_use] from serde_json and lazy_static.
  3. have a module called test_utils which is imported as test
    (basically use self::test_utils as test;) but that should be a different
    namspace, right?

Meta

Works with:

rustc 1.29.0 (aa3ca1994 2018-09-11)
binary: rustc
commit-hash: aa3ca1994904f2e056679fce1f185db8c7ed2703
commit-date: 2018-09-11
host: x86_64-unknown-linux-gnu
release: 1.29.0
LLVM version: 7.0

Doesn't work with

rustc 1.30.1 (1433507eb 2018-11-07)
binary: rustc
commit-hash: 1433507eba7d1a114e4c6f27ae0e1a74f60f20de
commit-date: 2018-11-07
host: x86_64-unknown-linux-gnu
release: 1.30.1
LLVM version: 8.0

Neither with

ustc 1.32.0-nightly (13c943992 2018-11-18)
binary: rustc
commit-hash: 13c9439925797cd7a65c917d047c07a500d9bfe6
commit-date: 2018-11-18
host: x86_64-unknown-linux-gnu
release: 1.32.0-nigh
@rustonaut

This comment has been minimized.

Copy link

rustonaut commented Nov 30, 2018

Minimal example:
(cut out from https://github.com/rustonaut/regression_test_attribute)

use self::test_utils as test;

mod test_utils;

#[test]
fn some_test() {

}
@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Dec 29, 2018

Fully minimized:

use std as attr;

#[attr] //~ ERROR cannot determine resolution for the attribute macro `attr`
fn main() {}

This code currently behaves as expected, namely we have a deadlock.
We cannot resolve and expand #[attr] because it's not known whether use std as attr imports a macro named attr or not.
Vice versa, we don't know whether use std as attr imports a macro named attr or not because #[attr] fn main() {} can produce a macro std after expansion.

Perhaps such deadlocks can be resolved by enforcing some additional shadowing restrictions (e.g. only if #[attr] indeed expands into a macro std then we report an error), but I'm not sure what are those necessary shadowing restrictions exactly.

@petrochenkov petrochenkov removed their assignment Dec 29, 2018

@petrochenkov petrochenkov removed the C-bug label Dec 29, 2018

@nikomatsakis nikomatsakis added T-lang and removed T-compiler labels Jan 10, 2019

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 10, 2019

Based on @petrochenkov's comment, I'm relabeling this as T-lang, since it seems like a "policy question", and I'm going to move to close as "expected fallout from the macro transition".

@rfcbot fcp close

@rfcbot

This comment has been minimized.

Copy link

rfcbot commented Jan 10, 2019

Team member @nikomatsakis has proposed to close this. The next step is review by the rest of the tagged teams:

Concerns:

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 10, 2019

@rfcbot concern none-of-us-in-this-meeting-truly-understand

@petrochenkov, upon re-reading, I feel some confusion about your examples. Let me first try to restate your "minimized example":

use std as attr;

#[attr] //~ ERROR cannot determine resolution for the attribute macro `attr`
fn main() {}

Here, I believe you are saying:

  • the #[attr] fn main() { } item could potentially generate an item named std
  • if it did so, then use std as attr could potentially import from it
    • therefore, we can't resolve use std as attr to a specific thing

Does that sound right? If so, I can certainly see why trying to resolve the name use std results in an ambiguity.

(That said, it is also true that, if we generated something shadowing std, that would generate an ambiguity error, right? Might that be the basis for a decision here?)

Regardless, I feel like this same reasoning doesn't apply to the minimal example given by @rustonaut. In particular, in that case, the path in question is self::test_utils, and there is a mod test_utils. Is the concern that the #[test] fn some_test in that example might itself generate a test_utils as well? (That case, too, would presumably be an error).

In particular I feel sort of confused about the "contours" of this ambiguity -- what are the cases where we have use foo as bar and #[bar] fn baz() and we are actually able to resolve? (If any)

@petrochenkov petrochenkov self-assigned this Jan 10, 2019

@Centril Centril removed the I-nominated label Jan 10, 2019

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Jan 11, 2019

@nikomatsakis

the #[attr] fn main() { } item could potentially generate an item named std

Correct.

if it did so, then use std as attr could potentially import from it

Correct.

If so, I can certainly see why trying to resolve the name use std results in an ambiguity.

It's more a "cannot make progress" error than ambiguity error.

That said, it is also true that, if we generated something shadowing std, that would generate an ambiguity error, right?

I don't understand this part. We are interested in std in macro namespace here, so we could give a definite answer to the question "does use std as attr import anything in macro namespace?".
There are no other things named std in scope in macro namespace except those potentially generated by #[attr] ....

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Jan 11, 2019

Regardless, I feel like this same reasoning doesn't apply to the minimal example given by @rustonaut. In particular, in that case, the path in question is self::test_utils, and there is a mod test_utils. Is the concern that the #[test] fn some_test in that example might itself generate a test_utils as well? (That case, too, would presumably be an error).

In the @rustonaut's example #[test] fn some_test() {} can also generate test_utils in macro namespace after expansion, it won't conflict with mod test_utils, which is in type namespace.

what are the cases where we have use foo as bar and #[bar] fn baz() and we are actually able to resolve? (If any)

If we can prove that foo (or source of the import in general) is "definitely not resolved" in macro namespace, then we can ignore the import use foo as bar (which is in the inner, highest priority scope) and continue the search in outer scopes (e.g. prelude) where the bar should supposedly be found.

prefix::foo is "definitely not resolved" in namespace NS if prefix is resolved, has no items named foo inside it, no "incomplete" glob imports and no unexpanded macros.
foo (single-segment) is "definitely not resolved" if it's "definitely not resolved" in each scope it can come from (current block, parent blocks, current module, preludes, ...).

@petrochenkov

This comment has been minimized.

Copy link
Contributor

petrochenkov commented Jan 11, 2019

use foo as bar;

#[bar] fn baz()

can be resolved if foo in macro namespace can be found without expanding #[bar] fn baz(), e.g. foo is in prelude, or is imported with macro_use, or is defined right in this module.
This usually means the intent of use foo as bar is indeed to import the macro and not something from another namespace.

If use foo as bar; doesn't intend to import-rename foo -> bar in macro namespace, but actually wants to do it in some other namespace, then there's no macro bar in scope most likely and we'll have a deadlock.

@petrochenkov petrochenkov removed their assignment Jan 11, 2019

@nikomatsakis

This comment has been minimized.

Copy link
Contributor

nikomatsakis commented Jan 14, 2019

@petrochenkov OK, thank you for the clarifications.

A key thing I think I was overlooking was the distinction of the macro vs type/module namespace.

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