[MIR trans] Translate statics #29759
Conversation
| // 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); |
There was a problem hiding this comment.
This may want to get somehow MIR-ified/updated as well because it causes an unnecessary alloca.
|
LGTM |
| // 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) |
There was a problem hiding this comment.
is this cast business just out of date?
There was a problem hiding this comment.
According to efriedman we do necessary casting sometime later in trans.
On Nov 11, 2015 4:01 PM, "Niko Matsakis" [email protected] 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.
|
I see now the conversation between @eefriedman and @nagisa. OK, LGTM! |
|
@bors r+ Thanks! |
|
📌 Commit f1342ff has been approved by |
|
⌛ Testing commit f1342ff with merge 0fd9ac5... |
|
💔 Test failed - auto-linux-64-nopt-t |
|
I have an approximate guess as to why SIGSEGV might happen. Previously 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 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 |
|
Huh. So I think that the |
|
@bors try |
|
Anyway, we can certainly do a try run, but that errors looks legit to me. |
|
@alexcrichton says he HAS seen this sort of crash before, though it occurs very rarely. |
|
@bors r- retry try |
Fixes #29578 r? @nikomatsakis My own observations are posted inline as comments.
|
If you are compiling without |
|
Try build seems to have succeeded (see towards the bottom of http://buildbot.rust-lang.org/grid?branch=try&refresh=15) |
|
@bors r+ let's give this another go. |
|
📌 Commit f1342ff has been approved by |
|
@bors try- retry r+ |
|
📌 Commit f1342ff has been approved by |
Fixes #29578 r? @nikomatsakis My own observations are posted inline as comments.
Fixes #29578
r? @nikomatsakis
My own observations are posted inline as comments.