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

duplicate symbols with enum in boxed closure #2074

Closed
grahame opened this issue Mar 29, 2012 · 7 comments
Closed

duplicate symbols with enum in boxed closure #2074

grahame opened this issue Mar 29, 2012 · 7 comments
Labels
E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added.

Comments

@grahame
Copy link
Contributor

grahame commented Mar 29, 2012

This program seems correct to me:

fn main(args: [str]) {
    let one = fn@() -> uint {
        enum r { a };
        ret a as uint;
    };
    let two = fn@() -> uint {
        enum r { a };
        ret a as uint;
    };
    one(); two();
}

If I compile it with rust 0d5d2e5 I get:

herugrim::~/code/limerick $ RUST_LOG=rustc=0,::rt::backtrace rustc enumbug.rs
enumbug.rs:2:0: 12:1 warning: unused variable args
enumbug.rs:2 fn main(args: [str]) {
enumbug.rs:3     let one = fn@() -> uint {
enumbug.rs:4         enum r { a };
enumbug.rs:5         ret a as uint;
enumbug.rs:6     };
enumbug.rs:7     let two = fn@() -> uint {
             ...
error: internal compiler error duplicate LLVM symbol: _ZN4main1r1a7discrim17_7bb11dc7a9aa7ee4E
rust: upcall fail 'explicit failure', ../src/rustc/driver/diagnostic.rs:78
rust: upcall fail 'explicit failure', ../src/rustc/driver/rustc.rs:182
rust: domain main @0x7fd822818600 root task failed

This problem only seems to happen with boxed closures (fn@)

@ghost ghost assigned catamorphism Apr 5, 2012
@catamorphism
Copy link
Contributor

Bug in how back::link::mangle_exported_name gets called (it gets called more than once with the same path and type, thus generates duplicate symbols like what you're seeing) -- fixing it right now.

@catamorphism
Copy link
Contributor

My fix was a red herring. This relates to #1344

@catamorphism
Copy link
Contributor

After a little more digging (and help from @brson ), here's what I figured about. The following code is similar, but compiles successfully:

fn main(args: [str]) {
    let one = fn@() -> uint {
      fn f() { fail; }
      ret 42u;
    };
    let two = fn@() -> uint {
      fn f() { fail; }
      ret 42u;
    };
    one(); two();
}

In this case, rustc does generate two symbols with the same name (for the two copies of f) and llvm silently mangles the names. In the enum case, rustc complains because trans_constant calls note_unique_llvm_symbol. The latter function is only called in a few places in trans. @graydon -- can you explain this? (This seems to be your code.)

@graydon
Copy link
Contributor

graydon commented Apr 6, 2012

Sure. It's there to catch this case, and oblige us to actually produce unique symbols. We need to invent a path prefix for the anonymous closure scope here.

The code you have "working" here should also call the unique symbol check.

(if we emit duplicate symbols, llvm silently renames them and breaks things worse)

@brson
Copy link
Contributor

brson commented Apr 8, 2012

I reverted c83d61d in order to solve some issues unrelated to this one.

@catamorphism
Copy link
Contributor

Reproduced as of a477c5a

@alexcrichton
Copy link
Member

This syntax is terrifyingly old, but this now passes so flagging as needstest

fn foo(args: ~[~str]) {          
    let one: @fn() -> uint = || {
        enum r { a }             
        a as uint                
    };                           
    let two: @fn() -> uint = || {
        enum r { a }             
        a as uint                
    };                           
    one(); two();                
}                                

fn main() {}

huonw added a commit to huonw/rust that referenced this issue Sep 16, 2013
bors added a commit that referenced this issue Sep 16, 2013
bors added a commit that referenced this issue Sep 17, 2013
@huonw huonw closed this as completed in 4f92f45 Sep 17, 2013
fhahn added a commit to fhahn/rust that referenced this issue Feb 12, 2014
* src/test/run-pass/issue-3559.rs was fixed in rust-lang#4726
* src/test/compile-fail/borrowck-call-sendfn.rs was fixed in rust-lang#2978
* update src/test/compile-fail/issue-5500-1.rs to work with current Rust
* removed src/test/compile-fail/issue-5500.rs because it is tested in
    src/test/run-fail/issue-5500.rs
* src/test/compile-fail/view-items-at-top.rs fixed
* rust-lang#897 fixed
* compile-fail/issue-6762.rs issue was closed as dup of rust-lang#6801
* deleted compile-fail/issue-2074.rs because it became irelevant and is
  irrelevant rust-lang#2074, a test covering this was added in
  4f92f45
fhahn added a commit to fhahn/rust that referenced this issue Feb 22, 2014
* compile-fail/vec-add.rs is obsolete, there are no mutable
  vectors any more, rust-lang#2711 is closed
* compile-fail/issue-1451.rs is obsolete, there are no more
  structural records, rust-lang#1451 is closed
* compile-fail/issue-2074.rs is obsolete, an up to date test
  is in run-pass/nested-enum-same-names.rs, rust-lang#2074 is closed
* compile-fail/omitted-arg-wrong-types.rs is obsolete, rust-lang#2093
  is closed
alexcrichton pushed a commit to alexcrichton/rust that referenced this issue Feb 25, 2014
* compile-fail/vec-add.rs is obsolete, there are no mutable
  vectors any more, rust-lang#2711 is closed
* compile-fail/issue-1451.rs is obsolete, there are no more
  structural records, rust-lang#1451 is closed
* compile-fail/issue-2074.rs is obsolete, an up to date test
  is in run-pass/nested-enum-same-names.rs, rust-lang#2074 is closed
* compile-fail/omitted-arg-wrong-types.rs is obsolete, rust-lang#2093
  is closed
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

Successfully merging a pull request may close this issue.

5 participants