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

mk: Replace 'compile_and_link' with 'oxidize' #12126

Merged
merged 1 commit into from
Feb 10, 2014
Merged

Conversation

brson
Copy link
Contributor

@brson brson commented Feb 9, 2014

I've always thought 'compile_and_link: ...' looked really awkward in our build output. Replace it with something more interesting. I'm open to alternatives to 'oxidize', like 'compile', anything but 'compile_and_link'.

bors added a commit that referenced this pull request Feb 10, 2014
I've always thought 'compile_and_link: ...' looked really awkward in our build output. Replace it with something more interesting. I'm open to alternatives to 'oxidize', like 'compile', anything but 'compile_and_link'.
@emberian
Copy link
Member

The amount of cheese is astounding.

@bors bors closed this Feb 10, 2014
@bors bors merged commit 3062d0f into rust-lang:master Feb 10, 2014
bors added a commit to rust-lang-ci/rust that referenced this pull request Jul 25, 2022
flip1995 pushed a commit to flip1995/rust that referenced this pull request Feb 27, 2024
Fix sign-handling bugs and false negatives in `cast_sign_loss`

**Note: anyone should feel free to move this PR forward, I might not see notifications from reviewers.**

changelog: [`cast_sign_loss`]: Fix sign-handling bugs and false negatives

This PR fixes some arithmetic bugs and false negatives in PR rust-lang#11883 (and maybe earlier PRs).
Cc `@J-ZhengLi`

I haven't updated the tests yet. I was hoping for some initial feedback before adding tests to cover the cases listed below.

Here are the issues I've attempted to fix:

#### `abs()` can return a negative value in release builds

Example:
```rust
i32::MIN.abs()
```
https://play.rust-lang.org/?version=stable&mode=release&edition=2021&gist=022d200f9ef6ee72f629c0c9c1af11b8

Docs: https://doc.rust-lang.org/std/primitive.i32.html#method.abs

Other overflows that produce negative values could cause false negatives (and underflows could produce false positives), but they're harder to detect.

#### Values with uncertain signs can be positive or negative

Any number of values with uncertain signs cause the whole expression to have an uncertain sign, because an uncertain sign can be positive or negative.

Example (from UI tests):
```rust
fn main() {
    foo(a: i32, b: i32, c: i32) -> u32 {
        (a * b * c * c) as u32
        //~^ ERROR: casting `i32` to `u32` may lose the sign of the value
    }

    println!("{}", foo(1, -1, 1));
}
```
https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=165d2e2676ee8343b1b9fe60db32aadd

#### Handle `expect()` the same way as `unwrap()`

Since we're ignoring `unwrap()` we might as well do the same with `expect()`.

This doesn't seem to have tests but I'm happy to add some like `Some(existing_test).unwrap() as u32`.

#### A negative base to an odd exponent is guaranteed to be negative

An integer `pow()`'s sign is only uncertain when its operants are uncertain. (Ignoring overflow.)

Example:
```rust
((-2_i32).pow(3) * -2) as u32
```

This offsets some of the false positives created by one or more uncertain signs producing an uncertain sign. (Rather than just an odd number of uncertain signs.)

#### Both sides of a multiply or divide should be peeled recursively

I'm not sure why the lhs was peeled recursively, and the rhs was left intact. But the sign of any sequence of multiplies and divides is determined by the signs of its operands. (Ignoring overflow.)

I'm not sure what to use as an example here, because most expressions I want to use are const-evaluable.

But if `p()` is [a non-const function that returns a positive value](https://doc.rust-lang.org/std/primitive.i32.html#method.isqrt), and if the lint handles unary negation, these should all lint:
```rust
fn peel_all(x: i32) {
    (-p(x) * -p(x) * -p(x)) as u32;
    ((-p(x) * -p(x)) * -p(x)) as u32;
    (-p(x) * (-p(x) * -p(x))) as u32;
}
```

#### The right hand side of a Rem doesn't change the sign

Unlike Mul and Div,
> Given remainder = dividend % divisor, the remainder will have the same sign as the dividend.
https://doc.rust-lang.org/reference/expressions/operator-expr.html#arithmetic-and-logical-binary-operators

I'm not sure what to use as an example here, because most expressions I want to use are const-evaluable.

But if `p()` is [a non-const function that returns a positive value](https://doc.rust-lang.org/std/primitive.i32.html#method.isqrt), and if the lint handles unary negation, only the first six expressions should lint.

The expressions that start with a constant should lint (or not lint) regardless of whether the lint supports `p()` or unary negation, because only the dividend's sign matters.

Example:
```rust
fn rem_lhs(x: i32) {
    (-p(x) % -1) as u32;
    (-p(x) % 1) as u32;
    (-1 % -p(x)) as u32;
    (-1 % p(x)) as u32;
    (-1 % -x) as u32;
    (-1 % x) as u32;
    // These shouldn't lint:
    (p(x) % -1) as u32;
    (p(x) % 1) as u32;
    (1 % -p(x)) as u32;
    (1 % p(x)) as u32;
    (1 % -x) as u32;
    (1 % x) as u32;
}
```

#### There's no need to bail on other expressions

When peeling, any other operators or expressions can be left intact and sent to the constant evaluator.

If these expressions can be evaluated, this offsets some of the false positives created by one or more uncertain signs producing an uncertain sign. If not, they end up marked as having uncertain sign.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants