Fix warnings when compiling stdlib with --test #30458
Conversation
|
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
fhahn
force-pushed
the
fix-warnings-tests-stdlib
branch
from
6d26750 to
a2019f2
Compare
|
Looks good to me, but this isn't my specialty in the codebase, so I'll let someone else r+ |
| @@ -59,6 +60,8 @@ pub fn init() { | |||
| assert!(signal(libc::SIGPIPE, libc::SIG_IGN) != !0); | |||
| } | |||
| } | |||
|
|
|||
| #[cfg(not(test))] | |||
| #[cfg(target_os = "nacl")] | |||
There was a problem hiding this comment.
I think these cfg annotations should be combined, right?
fhahn
force-pushed
the
fix-warnings-tests-stdlib
branch
from
a2019f2 to
d8a6eb5
Compare
|
@alexcrichton Thanks for your feedback! I've updated the commit. Now almost all warnings on Linux should be gone. The only remaining warnings are about the use of deprecated items in src/libstd/dynamic_lib.rs (https://github.com/rust-lang/rust/pull/30458/files#diff-59794c31034a5097419f3f376a30b4a4R135 and https://github.com/rust-lang/rust/pull/30458/files#diff-59794c31034a5097419f3f376a30b4a4R168), even though I've added |
fhahn
force-pushed
the
fix-warnings-tests-stdlib
branch
from
d8a6eb5 to
aecbb5d
Compare
| @@ -233,9 +233,8 @@ mod tests { | |||
| *lock.borrow_mut() = 1; | |||
| let lock2 = mc.lock().unwrap(); | |||
| *lock.borrow_mut() = 2; | |||
| let answer = Answer(lock2); | |||
| Answer(lock2); | |||
There was a problem hiding this comment.
I think this may have slightly changed the semantics here as Answer lives over the call to panic!, but changing the declaration to let _answer = ... and removing the drop should have the same semantics.
|
Thanks @fhahn! Looks good to me! |
fhahn
force-pushed
the
fix-warnings-tests-stdlib
branch
from
aecbb5d to
abe9b40
Compare
|
@alexcrichton thanks for your feedback! I've updated the PR again. |
|
@bors: r+ abe9b4088989912f0cfa2348e6a103131791f3df Thanks! |
|
⌛ Testing commit abe9b40 with merge efa0caf... |
|
💔 Test failed - auto-linux-64-x-android-t |
fhahn
force-pushed
the
fix-warnings-tests-stdlib
branch
from
abe9b40 to
217bebb
Compare
|
I've update the PR to fix the fail on android |
|
@bors r+ |
|
📌 Commit 217bebb has been approved by |
|
⌛ Testing commit 217bebb with merge 1d71545... |
|
💔 Test failed - auto-linux-64-x-android-t |
|
The build on Android fails, because I marked |
|
I agree they should match on Android too, but I think it could be done on a separate PR. |
fhahn
force-pushed
the
fix-warnings-tests-stdlib
branch
from
217bebb to
e27cbef
Compare
|
I've excluded the test from Android for now (at least I hope) and opened a PR for libc. After this lands, we should enable the test for android again. |
|
@bors r+ |
|
📌 Commit e27cbef has been approved by |
This PR siliences some warnings when compiling stdlib with --test. Mostly remove some unused imports and added a few `#[allow(..)]`. I also marked some signal handling functions with `#[cfg(not(test))]`, because they are only called through `rt::lang_start`, which is also marked as `#[cfg(not(test))]`
This PR siliences some warnings when compiling stdlib with --test. Mostly remove some unused imports and added a few
#[allow(..)].I also marked some signal handling functions with
#[cfg(not(test))], because they are only called throughrt::lang_start, which is also marked as#[cfg(not(test))]