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

faster fails to migrate #51211

Closed
nikomatsakis opened this issue May 30, 2018 · 6 comments
Closed

faster fails to migrate #51211

nikomatsakis opened this issue May 30, 2018 · 6 comments
Labels
A-edition-2018-lints Area: lints supporting the 2018 edition A-rust-2018-preview Area: The 2018 edition preview T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management

Comments

@nikomatsakis
Copy link
Contributor

@AdamNiederer helpfully reported that faster fails to migrate successfully:

Failing: https://github.com/AdamNiederer/faster/commits/rust-2018-migration

$ cargo build
   Compiling faster v0.4.3
error[E0463]: can't find crate for `core_or_std`
  --> src/intrin/eq.rs:58:25
   |
58 |                       use core_or_std::mem::transmute;
   |                           ^^^^^^^^^^^ can't find crate
...
79 | / rust_fallback_eq! {
80 | |     impl Eq for u8x16 where "sse2" {
81 | |         eq_mask, eq => u8x16, u8, _mm_cmpeq_epi8(), [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15];
82 | |     }
83 | | }
   | |_- in this macro invocation

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: Could not compile `faster`.

To learn more, run the command again with --verbose.

They write:

It looks like cargo fix isn't applying a crate:: prefix to my custom core/std module in a few cases. Manually fixing the imports fixes the build. It looks like cargo fix makes no attempt to fix imports in test modules, so the tests are still broken.

@nikomatsakis nikomatsakis added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management A-rust-2018-preview Area: The 2018 edition preview labels May 30, 2018
@Mark-Simulacrum
Copy link
Member

Possibly a case of #48704.

@alexcrichton alexcrichton added the A-edition-2018-lints Area: lints supporting the 2018 edition label Jul 10, 2018
@alexcrichton alexcrichton added this to the Rust 2018 Preview 2 milestone Jul 11, 2018
@alexcrichton
Copy link
Member

I've minimized this to the following:

#![feature(rust_2018_preview)]
#![warn(rust_2018_compatibility)]

macro_rules! foo {
    () => {
        {
            use myself::f;
            f();
        }
    }
}

mod myself {
    pub fn f() {}
}

fn main() {
    foo!();
    foo!();
}

which yields:

warning: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
  --> foo.rs:7:17
   |
7  |             use myself::f;
   |                 ^^^^^^^^^ help: use `crate`: `crate::myself::f`
...
18 |     foo!();
   |     ------- in this macro invocation
   |
note: lint level defined here
  --> foo.rs:2:9
   |
2  | #![warn(rust_2018_compatibility)]
   |         ^^^^^^^^^^^^^^^^^^^^^^^
   = note: #[warn(absolute_paths_not_starting_with_crate)] implied by #[warn(rust_2018_compatibility)]
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
   = note: for more information, see issue TBD

warning: absolute paths must start with `self`, `super`, `crate`, or an external crate name in the 2018 edition
  --> foo.rs:7:17
   |
7  |             use myself::f;
   |                 ^^^^^^^^^ help: use `crate`: `crate::myself::f`
...
19 |     foo!();
   |     ------- in this macro invocation
   |
   = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in the 2018 edition!
   = note: for more information, see issue TBD

It's unclear to me whether this falls under the purview of #48704 or rustfix. The problem is that two duplicate suggestions are made and rustfix yields an error on this because it looks suspicious. I'm gonna send a fix to rustfix in the meantime as I've tested locally that it works and should at least help improve the situation.

2 similar comments
@alexcrichton

This comment has been minimized.

@alexcrichton

This comment has been minimized.

@alexcrichton
Copy link
Member

I've attempted a fix for this in rustfix with rust-lang/rustfix#131 which fixes this exact issue, and at least successfully got the faster crate to migrate!

@alexcrichton
Copy link
Member

Actually, closing this in favor of #51205 as testing has shown that issue is the only thing that blocks faster

alexcrichton added a commit to alexcrichton/rustfix that referenced this issue Jul 16, 2018
Currently the compiler suffers from bugs like rust-lang/rust#51211 where it will
emit duplicate suggestions for code that originates in a macro. This commit
allows rustfix to accept and process these suggestions by ignoring exact
duplicates of replacements. While not a great solution long term it should
hopefully be a non-harmful band-aid until rustc is fixed!
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2018-lints Area: lints supporting the 2018 edition A-rust-2018-preview Area: The 2018 edition preview T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management
Projects
None yet
Development

No branches or pull requests

3 participants