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

Primitive binops are translated wrong when the LHS is mutated #27054

Closed
arielb1 opened this issue Jul 15, 2015 · 10 comments · Fixed by #58743
Closed

Primitive binops are translated wrong when the LHS is mutated #27054

arielb1 opened this issue Jul 15, 2015 · 10 comments · Fixed by #58743
Labels
C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@arielb1
Copy link
Contributor

arielb1 commented Jul 15, 2015

STR

fn main() {
    let x = Box::new(0);
    println!("{}", *x + { drop(x); let _ = Box::new(main); 0 });
}

Expected Results

If this code compiles, it should print 0.

Actual Results

Garbage is printed (the address of main, in fact).

@eefriedman
Copy link
Contributor

Looks like this is specifically an issue with the implementation of expr::trans_binary: Rust semantics say the value is supposed to be immediately loaded into a temporary, but expr::trans just returns a pointer into the Box.

@alexcrichton
Copy link
Member

triage: I-nominated

@alexcrichton alexcrichton added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Aug 13, 2015
@eefriedman
Copy link
Contributor

Some more testcases:

fn main() {
    let x = Box::new(0);
    let mut y = 0;
    *{ drop(x); let _ = Box::new(main); &mut y } = *x;
    println!("{}", y);
}
#[derive(Debug, Copy, Clone)]
struct CoolInt(i32);
impl std::ops::Add<CoolInt> for CoolInt {
    type Output = CoolInt;
    fn add(self, other: CoolInt) -> CoolInt { CoolInt(self.0 + other.0) }
}

fn main() {
    let x = Box::new(CoolInt(0));
    let mut y = CoolInt(0);
    println!("{:?}", *x + *{ drop(x); let _ = Box::new(main); &mut y });
}
struct R { x: i32, y: i32, z: i32 }
fn main() {
    let x = Box::new(0);
    let r = R { x: 0, y: 0, z: 0 };
    let r = R { x: *x, y: { drop(x); let _ = Box::new(main); 0 }, ..r};
    println!("{} {} {}", r.x, r.y, r.z);
}
fn main() {
    let x = Box::new(0);
    let mut y = 0;
    *{ drop(x); &mut y } += *x;
    println!("{}", y);
}

@arielb1
Copy link
Contributor Author

arielb1 commented Aug 18, 2015

@eefriedman

Your augmented assignment example is actually an order-of-operations mismatch - a different issue.

@eefriedman
Copy link
Contributor

@arielb1 Yes, I realized that later; filed #27868.

@nikomatsakis
Copy link
Contributor

Related to #28160

@nikomatsakis
Copy link
Contributor

triage: P-medium

@rust-highfive rust-highfive added P-medium Medium priority and removed I-nominated labels Sep 17, 2015
@eefriedman
Copy link
Contributor

Looks like this is fixed by MIR.

@Mark-Simulacrum
Copy link
Member

Mark-Simulacrum commented May 6, 2017

Verified as fixed by MIR, marking as E-needstest.

Edit: Well, verified as fixed. No idea if it was by MIR, though.

@Mark-Simulacrum Mark-Simulacrum added the E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. label May 6, 2017
@nikomatsakis
Copy link
Contributor

Yes, MIR would've fixed this.

@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. and removed I-wrong labels Jul 22, 2017
varkor added a commit to varkor/rust that referenced this issue Feb 26, 2019
Centril added a commit to Centril/rust that referenced this issue Feb 27, 2019
…hton

Add tests for several E-needstest issues

This PR adds a number of tests for various `E-needstest` errors. These tend to have been left open for a long time and seem unlikely to be closed otherwise.

Closes rust-lang#10876.
Closes rust-lang#22892.
Closes rust-lang#26448.
Closes rust-lang#26577.
Closes rust-lang#26619.
Closes rust-lang#27054.
Closes rust-lang#28587.
Closes rust-lang#44127.
Closes rust-lang#44255.
Closes rust-lang#55731.
Closes rust-lang#57781.
varkor added a commit to varkor/rust that referenced this issue Feb 27, 2019
pietroalbini added a commit to pietroalbini/rust that referenced this issue Mar 1, 2019
…hton

Add tests for several E-needstest issues

This PR adds a number of tests for various `E-needstest` errors. These tend to have been left open for a long time and seem unlikely to be closed otherwise.

Closes rust-lang#10876.
Closes rust-lang#22892.
Closes rust-lang#26448.
Closes rust-lang#26577.
Closes rust-lang#26619.
Closes rust-lang#27054.
Closes rust-lang#28587.
Closes rust-lang#44127.
Closes rust-lang#44255.
Closes rust-lang#55731.
Closes rust-lang#57781.
Centril added a commit to Centril/rust that referenced this issue Mar 9, 2019
…hton

Add tests for several E-needstest issues

This PR adds a number of tests for various `E-needstest` errors. These tend to have been left open for a long time and seem unlikely to be closed otherwise.

Closes rust-lang#10876.
Closes rust-lang#22892.
Closes rust-lang#26448.
Closes rust-lang#26577.
Closes rust-lang#26619.
Closes rust-lang#27054.
Closes rust-lang#28587.
Closes rust-lang#44127.
Closes rust-lang#44255.
Closes rust-lang#55731.
Closes rust-lang#57781.
Centril added a commit to Centril/rust that referenced this issue Mar 9, 2019
…hton

Add tests for several E-needstest issues

This PR adds a number of tests for various `E-needstest` errors. These tend to have been left open for a long time and seem unlikely to be closed otherwise.

Closes rust-lang#10876.
Closes rust-lang#22892.
Closes rust-lang#26448.
Closes rust-lang#26577.
Closes rust-lang#26619.
Closes rust-lang#27054.
Closes rust-lang#28587.
Closes rust-lang#44127.
Closes rust-lang#44255.
Closes rust-lang#55731.
Closes rust-lang#57781.
varkor added a commit to varkor/rust that referenced this issue Mar 11, 2019
bors added a commit that referenced this issue Mar 12, 2019
Add tests for several E-needstest issues

This PR adds a number of tests for various `E-needstest` errors. These tend to have been left open for a long time and seem unlikely to be closed otherwise.

Closes #10876.
Closes #26448.
Closes #26577.
Closes #26619.
Closes #27054.
Closes #44127.
Closes #44255.
Closes #55731.
Closes #57781.
varkor added a commit to varkor/rust that referenced this issue Mar 12, 2019
pietroalbini added a commit to pietroalbini/rust that referenced this issue Mar 12, 2019
…hton

Add tests for several E-needstest issues

This PR adds a number of tests for various `E-needstest` errors. These tend to have been left open for a long time and seem unlikely to be closed otherwise.

Closes rust-lang#10876.
Closes rust-lang#26448.
Closes rust-lang#26577.
Closes rust-lang#26619.
Closes rust-lang#27054.
Closes rust-lang#44127.
Closes rust-lang#44255.
Closes rust-lang#55731.
Closes rust-lang#57781.
bors added a commit that referenced this issue Mar 12, 2019
Add tests for several E-needstest issues

This PR adds a number of tests for various `E-needstest` errors. These tend to have been left open for a long time and seem unlikely to be closed otherwise.

Closes #10876.
Closes #26448.
Closes #26577.
Closes #26619.
Closes #27054.
Closes #44127.
Closes #44255.
Closes #55731.
Closes #57781.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. E-needs-test Call for participation: An issue has been fixed and does not reproduce, but no test has been added. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants