-
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
[MIR] Initial implementation for translating calls. #30364
Conversation
r? @jroesch (rust_highfive has picked a reviewer for you, use r? to override) |
@@ -49,8 +68,11 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> { | |||
{ | |||
let constant_ty = bcx.monomorphize(&constant.ty); | |||
match constant.literal { | |||
mir::Literal::Item { .. } => { | |||
unimplemented!() | |||
mir::Literal::Item { def_id, substs } => { |
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.
Wait until #29907 is landed?
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.
Yea sure, ended up seeing it later and it's definitely nicer and more complete. I'll rebase on top of that instead.
fn test2(a: isize) -> isize { | ||
// Test passing a single argument. | ||
// Not using out pointer. | ||
fn callee(a: isize) -> isize { |
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.
can you add some tests using trait dispatch? (or does that not work yet)
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.
Added some more tests for calling methods in different ways. #29907 + this pull also allow calling trait static methods which i've also included a test for.
☔ The latest upstream changes (presumably #29907) made this pull request unmergeable. Please resolve the merge conflicts. |
So #29907 has landed. I see that this PR was rebased, presumably to account for that? |
📌 Commit fefe4e8 has been approved by |
⌛ Testing commit fefe4e8 with merge b784e6c... |
💔 Test failed - auto-win-msvc-64-opt |
@luqmana That failure on windows is legit. The call to
|
See #29855 |
As @nagisa said on IRC, I think it's fine to just remove the |
On Fri, Dec 18, 2015 at 08:20:02AM -0800, Simonas Kazlauskas wrote:
Oh yeah, we probably want to modify trans to zero. Bah. Forgot about |
@bors: r=nikomatsakis Just removed Box from the test for now til we get drop working. |
📌 Commit a6b861b has been approved by |
#29575