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

ensure that the assert! macro doesn't ICE on non-booleans #31604

Closed
wants to merge 3 commits into from

Conversation

oli-obk
Copy link
Contributor

@oli-obk oli-obk commented Feb 12, 2016

cc #31592

Note that this is not a fix, it exposes the span-bug even more. During stage1 command-before-exec.rs fails to run, because

{...}x86_64-unknown-linux-gnu/test/run-pass/command-before-exec.stage1-x86_64-unknown-linux-gnu: symbol lookup error: {...}x86_64-unknown-linux-gnu/test/run-pass/command-before-exec.stage1-x86_64-unknown-linux-gnu: undefined symbol: _ZN6option15Option$LT$T$GT$6unwrap14_MSG_FILE_LINE20hc561c4c240f9e4ddnxOE

it works fine in stage2


This cannot be done for assert_eq, because there's no way I couldn't find a way to ensure two expressions have the same types without moving them into an array. decltype ftw

@rust-highfive
Copy link
Collaborator

r? @sfackler

(rust_highfive has picked a reviewer for you, use r? to override)

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 12, 2016

@alexcrichton You created the before_exec function. Any thoughts?

@ollie27
Copy link
Member

ollie27 commented Feb 12, 2016

This will break this code:

use std::ops::Not;

struct Foo;

impl Not for Foo {
    type Output = bool;

    fn not(self) -> Self::Output {
        false
    }
}

fn main() {
    assert!(Foo);
}

I don't know if that breakage is acceptable but in this case it can be fixed by putting the ! back in front of the $cond.

@oli-obk
Copy link
Contributor Author

oli-obk commented Feb 12, 2016

ugh :( I so want to break this code. But that's probably not acceptable breakage.

@ollie27
Copy link
Member

ollie27 commented Feb 12, 2016

Yeah it's a similar situation to #30143.

@oli-obk oli-obk closed this Feb 12, 2016
@oli-obk oli-obk deleted the fix/assert branch February 22, 2016 10:38
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