-
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
librustc_llvm: encapsulate "as" cast from/to a bool #32700
Conversation
r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) |
@@ -2720,7 +2720,7 @@ pub fn trans_crate<'tcx>(tcx: &TyCtxt<'tcx>, | |||
static INIT: Once = Once::new(); | |||
static mut POISONED: bool = false; | |||
INIT.call_once(|| { | |||
if llvm::LLVMStartMultithreaded() != 1 { | |||
if ! llvm::as_bool(llvm::LLVMStartMultithreaded()) { |
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.
Stylistically we tend to not have spaces after the !
operator
This seems like it may be a difficult convention to follow as it's easier to just call the function directly and not call an extra function to wrap the result. How do we ensure that we avoid adding more |
When the trueness is defined as "!= 0" rather than "== 1", it's error prone to write such a comparison for each occurrence. Signed-off-by: NODA, Kai <nodakai@gmail.com>
bda2358
to
501d61a
Compare
@alexcrichton True, when the underlying APIs themselves have a confusing interface, I can only hope someone modifying this potion of code notices the |
I guess my point is that I don't think this is worth it unless we have a deeper change. We're not currently in the mindset of avoiding |
☔ The latest upstream changes (presumably #31709) made this pull request unmergeable. Please resolve the merge conflicts. |
Should this be closed? |
No. 野田 開 nodakai@gmail.com 2016-05-04 1:19 GMT+08:00 Simonas Kazlauskas notifications@github.com:
|
Closing due to inactivity, but feel free to reopen with a rebase! |
When the trueness is defined as "!= 0" rather than "== 1", it's error prone to write such a comparison for each occurrence.