-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Represent function pointers in mir-constants as a Value instead of Item #40602
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except the hacks can be removed now!
src/librustc_trans/mir/constant.rs
Outdated
// Shortcut for zero-sized types, including function item | ||
// types, which would not work with MirConstContext. | ||
// Shortcut for zero-sized types | ||
// which would not work with MirConstContext. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this means it's a function-specific hack that can be removed.
src/librustc_trans/mir/constant.rs
Outdated
@@ -927,8 +930,8 @@ impl<'a, 'tcx> MirContext<'a, 'tcx> { | |||
let ty = self.monomorphize(&constant.ty); | |||
let result = match constant.literal.clone() { | |||
mir::Literal::Item { def_id, substs } => { | |||
// Shortcut for zero-sized types, including function item | |||
// types, which would not work with MirConstContext. | |||
// Shortcut for zero-sized types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
Gotta run, so I can't test the changes locally. let's see what travis says. |
@bors r+ |
📌 Commit 187a922 has been approved by |
Represent function pointers in mir-constants as a Value instead of Item r? @eddyb
☔ The latest upstream changes (presumably #39628) made this pull request unmergeable. Please resolve the merge conflicts. |
You have a merge commit, please rebase properly. |
since the MIR-shim changes I can't figure out how to make this work. |
I applied the fixes as discussed in IRC. It works now. |
@bors r+ |
📌 Commit 9c91846 has been approved by |
Represent function pointers in mir-constants as a Value instead of Item r? @eddyb
r? @eddyb