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

Import regression with use_extern_macros #50725

Closed
petrochenkov opened this issue May 14, 2018 · 1 comment
Closed

Import regression with use_extern_macros #50725

petrochenkov opened this issue May 14, 2018 · 1 comment
Assignees
Labels
A-resolve Area: Name resolution

Comments

@petrochenkov
Copy link
Contributor

petrochenkov commented May 14, 2018

Found by running our test suite with use_extern_macros enabled.

With use_extern_macros disabled:

use std::panic::{self, AssertUnwindSafe}; // OK

fn main() {
    panic!();
}

With use_extern_macros enabled:

#![feature(use_extern_macros)]

use std::panic::{self, AssertUnwindSafe}; // ERROR `self` no longer imports values

fn main() {
    panic!(); // Curiously, the error disappears if this panic is removed
}

I haven't investigated what happens yet.
use a::b::{self}; should filter away all namespaces except for type, and it should report an error only if nothing is left.
In the example with use std::panic::{self}; above only the panic module should be imported (without an error).

@petrochenkov
Copy link
Contributor Author

#50760 partially fixes this, but here is an example that still causes an error:

#![feature(use_extern_macros)]

use std::panic::{self};

fn main() {
    assert!(true); //~ ERROR expected (), found bool
}

bors added a commit that referenced this issue May 19, 2018
Turn deprecation lint `legacy_imports` into a hard error

Closes #38260

The lint was introduced in Dec 2016, then made deny-by-default in Jun 2017 when crater run found 0 regressions caused by it.

This lint requires some not entirely trivial amount of import resolution logic that (surprisingly or not) interacts with `feature(use_extern_macros)` (#35896), so it would be desirable to remove it before stabilizing `use_extern_macros`.
In particular, this PR fixes the failing example in #50725 (but not the whole issue, `use std::panic::{self}` still can cause other undesirable errors when `use_extern_macros` is enabled).
@petrochenkov petrochenkov self-assigned this May 19, 2018
bors added a commit that referenced this issue May 20, 2018
resolve: Don't add unnecessary import candidates for `prefix::{self}` imports

Fixes #50725
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-resolve Area: Name resolution
Projects
None yet
Development

No branches or pull requests

1 participant