Skip to content

Fix warnings when compiling stdlib with --test - #30458

Merged
bors merged 1 commit into
rust-lang:masterfrom
fhahn:fix-warnings-tests-stdlib
Dec 30, 2015
Merged

Fix warnings when compiling stdlib with --test#30458
bors merged 1 commit into
rust-lang:masterfrom
fhahn:fix-warnings-tests-stdlib

Conversation

@fhahn

Copy link
Copy Markdown
Contributor

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))]

@rust-highfive

Copy link
Copy Markdown
Contributor

r? @aturon

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

@fhahn
fhahnforce-pushed the fix-warnings-tests-stdlib branch from 6d26750 to a2019f2CompareDecember 18, 2015 12:51
@steveklabnik

Copy link
Copy Markdown
Contributor

Looks good to me, but this isn't my specialty in the codebase, so I'll let someone else r+

Comment threadsrc/libstd/sys/unix/mod.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these cfg annotations should be combined, right?

@fhahn
fhahnforce-pushed the fix-warnings-tests-stdlib branch from a2019f2 to d8a6eb5CompareDecember 20, 2015 23:53
@fhahn

Copy link
Copy Markdown
ContributorAuthor

@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 #[allow(deprecated)].

@fhahn
fhahnforce-pushed the fix-warnings-tests-stdlib branch from d8a6eb5 to aecbb5dCompareDecember 21, 2015 00:00
Comment threadsrc/libstd/sys/common/remutex.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@alexcrichton

Copy link
Copy Markdown
Member

Thanks @fhahn! Looks good to me!

@fhahn
fhahnforce-pushed the fix-warnings-tests-stdlib branch from aecbb5d to abe9b40CompareDecember 22, 2015 13:03
@fhahn

Copy link
Copy Markdown
ContributorAuthor

@alexcrichton thanks for your feedback! I've updated the PR again.

@alexcrichton

Copy link
Copy Markdown
Member

@bors: r+ abe9b4088989912f0cfa2348e6a103131791f3df

Thanks!

@bors

bors commented Dec 23, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit abe9b40 with merge efa0caf...

@bors

bors commented Dec 23, 2015

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-linux-64-x-android-t

@fhahn
fhahnforce-pushed the fix-warnings-tests-stdlib branch from abe9b40 to 217bebbCompareDecember 24, 2015 13:22
@fhahn

Copy link
Copy Markdown
ContributorAuthor

I've update the PR to fix the fail on android

@sanxiyn

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Dec 28, 2015

Copy link
Copy Markdown
Collaborator

📌 Commit 217bebb has been approved by sanxiyn

@bors

bors commented Dec 28, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 217bebb with merge 1d71545...

@bors

bors commented Dec 28, 2015

Copy link
Copy Markdown
Collaborator

💔 Test failed - auto-linux-64-x-android-t

@fhahn

Copy link
Copy Markdown
ContributorAuthor

The build on Android fails, because I marked os::raw::tests::unix as test. I could disable it again for Android (or in general), but shouldn"t the types of nlink_t in libc and os::raw be equal?

@sanxiyn

Copy link
Copy Markdown
Contributor

I agree they should match on Android too, but I think it could be done on a separate PR.

@fhahn
fhahnforce-pushed the fix-warnings-tests-stdlib branch from 217bebb to e27cbefCompareDecember 29, 2015 15:07
@fhahn

Copy link
Copy Markdown
ContributorAuthor

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.

@sanxiyn

Copy link
Copy Markdown
Contributor

@bors r+

@bors

bors commented Dec 30, 2015

Copy link
Copy Markdown
Collaborator

📌 Commit e27cbef has been approved by sanxiyn

@bors

bors commented Dec 30, 2015

Copy link
Copy Markdown
Collaborator

⌛ Testing commit e27cbef with merge a06bb97...

bors added a commit that referenced this pull request Dec 30, 2015
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))]`
@bors
bors merged commit e27cbef into rust-lang:masterDec 30, 2015
@fhahn
fhahn deleted the fix-warnings-tests-stdlib branch December 30, 2015 15:40
Sign up for freeto 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.

7 participants

@fhahn@rust-highfive@steveklabnik@alexcrichton@bors@sanxiyn@aturon