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 "no enclosing scope" with cross-crate map #5521

Closed
alexcrichton opened this issue Mar 24, 2013 · 15 comments
Closed

ICE "no enclosing scope" with cross-crate map #5521

alexcrichton opened this issue Mar 24, 2013 · 15 comments
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@alexcrichton
Copy link
Member

Given these two files:

// foo.rs
#[link(name="foo",vers="0.0")];
#[crate_type = "lib"];
use std::hashmap::HashMap;

pub type map = @mut HashMap<uint, uint>;

// end of foo.rs
// bar.rs
extern mod foo;

fn foo(a: foo::map) {
    if false {
        fail!();
    } else {
        let _b = a.get(&2); // error may for this line; doesn't happen if "let _b" is omitted
    }
}

fn main() {}

// end of bar.rs

Yield this result:

$ rustc foo.rs && rustc -L . bar.rs
warning: no debug symbols in executable (-arch x86_64)
error: internal compiler error: no enclosing scope with id 18
bors added a commit that referenced this issue Mar 26, 2013
I started out just removing a few instances of `HashMap` throughout rustc, but it ended up snowballing to remove the entire thing. Most uses translated to just using `@mut LinearMap` instead of `HashMap`, although I tried where possible to drop the `@mut` modifier. This ended up working out some of the time, but definitely not in the major use cases.

Things got kinda weird in some cases like:

* alexcrichton/rust@mozilla:a56ec8c1342453a88be79e192a11501844375d40...alexcrichton:621b63300358cacad088ddd7f78180f29c40e66e#L39R1587
* alexcrichton/rust@mozilla:a56ec8c1342453a88be79e192a11501844375d40...alexcrichton:621b63300358cacad088ddd7f78180f29c40e66e#L61R3760
* alexcrichton/rust@mozilla:a56ec8c1342453a88be79e192a11501844375d40...alexcrichton:621b63300358cacad088ddd7f78180f29c40e66e#L71R917
* alexcrichton/rust@mozilla:a56ec8c1342453a88be79e192a11501844375d40...alexcrichton:621b63300358cacad088ddd7f78180f29c40e66e#L91R127

I tried to tag them all with bugs which I thought would make them less weird, but I may have the wrong bug in a few places. These cases only came up when I tried to pass around `&mut LinearMap` instead of an `@mut LinearMap`.

I also ran into a few bugs when migrating to `LinearMap`, one of which is #5521. There's another set of bugs which a00d779042fb8753c716e07b4f1aac0d5ab7bf66 addresses (all marked with `XXX`). I have a feeling they're all the same bug, but all I've been able is to reproduce them. I tried to whittle down the test cases and try to get some input which causes a failure, but I've been unable to do so. All I know is that it's vaguely related to `*T` pointers being used as `&*T` (return value of `find`). I'm not able to open a very descriptive issue, but I'll do so if there seems no other better route.

I realize this is a very large pull request, so if it'd be better to split this up into multiple segments I'd be more than willing to do so. So far the tests all pass locally, although I'm sure bors will turn something up. I also don't mind keeping this up to date with rebasing. This maybe should wait until after 0.6 because it is a fairly large change...
@catamorphism
Copy link
Contributor

Reproduced with 0252c30 . Nominating for milestone 5, production-ready.

@jbclements
Copy link
Contributor

related to #3860 ?

@graydon
Copy link
Contributor

graydon commented May 23, 2013

accepted for production-ready milestone

@graydon
Copy link
Contributor

graydon commented May 23, 2013

relative of #4539

@nikomatsakis
Copy link
Contributor

Dup / consequence of #4539

@pnkfelix
Copy link
Member

Visiting for triage email 2013-07-22. ICE still occurs.

@nikomatsakis when you said its a "Dup / consequence of #4539", which is a pull request (PR) that has been closed, did you mean that this bug was injected by the code in that PR? Or something that should have been fixed by that PR and was not? I'm just trying to understand how one can claim that the two are duplicates when one is closed and other is clearly reproducible.

@pnkfelix
Copy link
Member

(perhaps @nikomatsakis 's comment is related to his feedback on PR #4539 ...)

@nikomatsakis
Copy link
Contributor

tbh I'm not entirely sure what I meant anymore. @pnkfelix can you update the example (I presume you have an updated vesion lying around) and I will re-investigate?

@pnkfelix
Copy link
Member

@nikomatsakis yeah no problem, I forgot that I did have to do a teensy bit of updating.

@pnkfelix
Copy link
Member

(oh wait, this might be fixed now; my earlier tests were using a build from July 10th. double-checking now.)

@pnkfelix
Copy link
Member

@nikomatsakis I updated the test case in the description.

The bug became more difficult to expose sometime between July 16 (commit a317584 ) and July 17 (commit a93244d ), based on a coarse bisection.

I found a small perturbation of the source that continues to expose the bug.

(The original version had the line that exposes the bug in dead code before; I suspect that commit 4797dd4 may thus have masked the bug. The interesting thing is that changes such as copying the same line into both branches of the if-statement also masked the bug, as does inverting the branch-order (and condition to match), which should be a semantics-preserving transformation; some of these masking-effects only started becoming effective also between July 16 and July 17th, which is really interesting.)

Anyway, the bug remains.

@nikomatsakis
Copy link
Contributor

I did make some changes in the new borrow checker work that should have addressed this bug, but they were not perfect. The proper fix is (I suspect, I haven't looked too closely) #6248

@alexcrichton
Copy link
Member Author

Sadly this bug still exists :(

@alexcrichton
Copy link
Member Author

This appears to be fixed. Or at least I can't write @mut any more and @ doesn't yield an error.

Flagging as needstest.

@nikomatsakis
Copy link
Contributor

This is quickly becoming irrelevant, I suspect, as we remove the special cases concerning @ from borrow checker and trans.

alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 3, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue May 27, 2020
Register redundant_field_names and non_expressive_names as early passes

Similar names was moved to a pre-expansion pass to solve rust-lang#2927, so I'm avoiding linting on code from expansion, which makes the dogfood (mostly, see below) pass.

I had to change new_without_default though, and although I understand why it was not triggering before, TBH I don't see why the binding inside the nested `if_chain` is being linted now. Any ideas? (it seems legit though as the code can be changed by the user)

changelog: Register redundant_field_names and non_expressive_names as early passes

Fixes rust-lang#5356
Fixes rust-lang#5521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.
Projects
None yet
Development

No branches or pull requests

6 participants