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

ICE when printing paths in modules with non-DefId reexports (e.g. NonMacroAttr(Builtin)). #74236

Closed
eddyb opened this issue Jul 11, 2020 · 1 comment
Labels
C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@eddyb
Copy link
Member

eddyb commented Jul 11, 2020

dep.rs (compiled with --edition=2018):

mod private { pub struct Pub; }
// Reexport built-in attribute without a DefId (requires Rust 2018).
pub use cfg_attr as attr;
// This export needs to be after the built-in attribute to trigger the bug.
pub use private::Pub as Renamed;

main.rs:

fn main() {
    // Trigger an error that will print the path of dep::private::Pub (as "dep::Renamed").
    let () = dep::Renamed;
}

Results in attempted .def_id() on invalid res: NonMacroAttr(Builtin) due to the .def_id() call in:

DefPathData::TypeNs(ref mut name) if Some(visible_parent) != actual_parent => {
let reexport = self
.tcx()
.item_children(visible_parent)
.iter()
.find(|child| child.res.def_id() == def_id)
.map(|child| child.ident.name);
if let Some(reexport) = reexport {
*name = reexport;
}
}

The snippet above is searching dep's children for the reexport of dep::private::Pub in order to grab its reexported name (Renamed here) when it runs into the pub use cfg_attr as attr; reexport lacking a DefId.

Fix is straight-forward, we can just ignore siblings lacking a DefId:

-         .find(|child| child.res.def_id() == def_id)
+         .find(|child| child.res.opt_def_id() == Some(def_id))

But I'm mostly making this issue to point out the fact that it is possible to create strange reexports that could break various parts of the compiler, even if it might be very rare (#74081 appears to be an instance of this).

(as I was writing this I found that @da-x had already tried out the fix - see #74081 (comment))

cc @rust-lang/compiler

@jonas-schievink jonas-schievink added C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ I-prioritize Issue: Indicates that prioritization has been requested for this issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 11, 2020
da-x added a commit to da-x/rust that referenced this issue Jul 11, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 11, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 11, 2020
Manishearth added a commit to Manishearth/rust that referenced this issue Jul 11, 2020
bors added a commit to rust-lang-ci/rust that referenced this issue Jul 12, 2020
…arth

Rollup of 10 pull requests

Successful merges:

 - rust-lang#72920 (Stabilize `transmute` in constants and statics but not const fn)
 - rust-lang#73715 (debuginfo:  Mangle tuples to be natvis friendly, typedef basic types)
 - rust-lang#74066 (Optimize is_ascii for str and [u8].)
 - rust-lang#74116 (Fix cross compilation of LLVM to aarch64 Windows targets)
 - rust-lang#74167 (linker: illumos ld does not support --eh-frame-hdr)
 - rust-lang#74168 (Add a help to use `in_band_lifetimes` in nightly)
 - rust-lang#74197 (Reword incorrect `self` token suggestion)
 - rust-lang#74213 (Minor refactor for rustc_resolve diagnostics match)
 - rust-lang#74240 (Fix rust-lang#74081 and add the test case from rust-lang#74236)
 - rust-lang#74241 (update miri)

Failed merges:

r? @ghost
@JohnTitor
Copy link
Member

#74240 fixed and added the test for this, closing as fixed.

@JohnTitor JohnTitor removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jul 14, 2020
roxelo pushed a commit to sexxi-goose/rust that referenced this issue Jul 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants