[MIR trans] Translate statics

[nagisa]

Fixes #29578

r? @nikomatsakis

My own observations are posted inline as comments.

[nagisa]

This may want to get somehow MIR-ified/updated as well because it causes an unnecessary alloca.

[eefriedman]

LGTM

[nikomatsakis]

is this cast business just out of date?

[nagisa]

According to efriedman we do necessary casting sometime later in trans.
On Nov 11, 2015 4:01 PM, "Niko Matsakis" notifications@github.com wrote:

In src/librustc_trans/trans/expr.rs
#29759 (comment):

         let const_ty = expr_ty(bcx, ref_expr);

•        // For external constants, we don't inline.
  

•        let val = if let Some(node_id) = bcx.tcx().map.as_local_node_id(did) {
  

- // Case 1.

•            // The LLVM global has the type of its initializer,
  

•            // which may not be equal to the enum's type for
  

•            // non-C-like enums.
  

•            let val = base::get_item_val(bcx.ccx(), node_id);
  

•            let pty = type_of::type_of(bcx.ccx(), const_ty).ptr_to();
  

•            PointerCast(bcx, val, pty)
  

is this cast business just out of date?

—
Reply to this email directly or view it on GitHub
https://github.com/rust-lang/rust/pull/29759/files#r44534266.

[nikomatsakis]

I see now the conversation between @eefriedman and @nagisa. OK, LGTM!

[nikomatsakis]

@bors r+

Thanks!

[bors]

📌 Commit f1342ff has been approved by nikomatsakis

[bors]

⌛ Testing commit f1342ff with merge 0fd9ac5...

[bors]

💔 Test failed - auto-linux-64-nopt-t

[nagisa]

I have an approximate guess as to why SIGSEGV might happen. Previously get_static_val used to call trans_external_path and were static contain a function in it, it would SIGSEGV LLVM, so I changed trans_external_path to just trans_extern_const – its a get_static_val after all.

However, I think that might be a reason for us miscompiling something somewhere in libstd and therefore this generalisation proposed by @eefriedman cannot be applied, at least at the moment. I’ll revert the generalisation and use the regular PointerCastey code in MIR as well if this turns out to be true.

---

That being said, I cannot reproduce this failure locally (and the test that seems to run next doesn’t seem to be relevant at all env::tests::join_paths_unix), therefore I’d like a try run or re-r+ before I revert if this was, perchance, some natural phenomenon at work.

[nikomatsakis]

Huh. So I think that the PointerCast would not lead to a SEGV, but rather a type error from LLVM -- although it may be that we run without LLVM's integrity assertions or something, which could cause a SEGV I gues.

[nikomatsakis]

@bors try

[nikomatsakis]

Anyway, we can certainly do a try run, but that errors looks legit to me.

[nikomatsakis]

@alexcrichton says he HAS seen this sort of crash before, though it occurs very rarely.

[nikomatsakis]

@bors r- retry try

[bors]

⌛ Trying commit f1342ff with merge 2adb618...

[arielb1]

@nikomatsakis

If you are compiling without --enable-llvm-assertions, LLVM assertion failures cause a core dump.

[nagisa]

Try build seems to have succeeded (see towards the bottom of http://buildbot.rust-lang.org/grid?branch=try&refresh=15)

[nikomatsakis]

@bors r+

let's give this another go.

[bors]

📌 Commit f1342ff has been approved by nikomatsakis

[nikomatsakis]

@bors try- retry r+

[bors]

📌 Commit f1342ff has been approved by nikomatsakis

[bors]

⌛ Testing commit f1342ff with merge 3beb159...

[bors]

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-nopt-t, auto-mac-64-opt, auto-win-gnu-32-nopt-t, auto-win-gnu-32-opt, auto-win-gnu-64-nopt-t, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-opt