Skip to content

Remove dead code from sys::windows::c #31782

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

Merged
merged 3 commits into from
Feb 24, 2016
Merged

Remove dead code from sys::windows::c #31782

merged 3 commits into from
Feb 24, 2016

Conversation

pitdicker
Copy link
Contributor

I am not entirely sure I have got everything right, but if it compiles it is ok probably...
I tested it with msvc x86_64 and gnu.

Somehow a lot of EXCEPTION-* constants are dead code when running test, no idea why.
I have put #![cfg_attr(test, allow(dead_code))] at the top for this.

@rust-highfive
Copy link
Contributor

r? @alexcrichton

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

@alexcrichton
Copy link
Member

@bors: r+ 0a6f35b

Quite the cleanup!

@bors
Copy link
Collaborator

bors commented Feb 21, 2016

⌛ Testing commit 0a6f35b with merge a2ee00f...

@bors
Copy link
Collaborator

bors commented Feb 21, 2016

💔 Test failed - auto-win-gnu-32-nopt-t

@pitdicker
Copy link
Contributor Author

I can't really get cross-compiling set up, but I think this fixes it for 32-bit.
Should I somehow first test myself, or is it ok to try?

@alexcrichton
Copy link
Member

@bors: r+ 94db864

If this bounces I've got a simple script to just mock compile the repo, which may be useful for ensuring that other platforms just compile

@pitdicker
Copy link
Contributor Author

Thanks!

I have no idea what that script does...

@alexcrichton
Copy link
Member

Ah if you tweak the first two lines to uncomment the right stage/triple and just run it inside of a checkout of the compiler it will "bootstrap" that target into a directory called out

@bors
Copy link
Collaborator

bors commented Feb 21, 2016

⌛ Testing commit 94db864 with merge b8e2d98...

@bors
Copy link
Collaborator

bors commented Feb 21, 2016

💔 Test failed - auto-win-gnu-32-nopt-t

@pitdicker
Copy link
Contributor Author

mingw really does not like me... I have reinstalled msys and 64- and 32-bit mingw and made a fresh checkout and rebuild of rust. But cross-compiling seems out of reach for me. And I tried your script.
Is there any guide for setting this it up?

This pr should work now, but untested for 32-bit.

@alexcrichton
Copy link
Member

I tested this out locally and got (for the MSVC targets)

src/libstd/sys/windows/c.rs:1090:5: 1093:58 warning: foreign function is never used: `RaiseException`, #[warn(dead_code)] on by default
src/libstd/sys/windows/c.rs:1090     pub fn RaiseException(dwExceptionCode: DWORD,
src/libstd/sys/windows/c.rs:1091                           dwExceptionFlags: DWORD,
src/libstd/sys/windows/c.rs:1092                           nNumberOfArguments: DWORD,
src/libstd/sys/windows/c.rs:1093                           lpArguments: *const ULONG_PTR);

Both the MinGW targets compiled with no warnings, however.

@pitdicker
Copy link
Contributor Author

Thanks so much for testing this! A little cleanup is not worth all the effort this takes...

I don't understand why RaiseException is not used on msvc. Both msvc targets use seh.rs.
And seh.rs uses it, except for stage0.
But I thought dead code in stage0 only gives warnings, not errors?

@retep998
Copy link
Member

@pitdicker Which is why the message says warning: foreign function is never used: RaiseException, #[warn(dead_code)] on by default, which is a warning and not an error.

@pitdicker
Copy link
Contributor Author

@retep998 The problem is the later stages deny warnings. So I am not sure if compiling fails with this warning, and if so, why?

@alexcrichton
Copy link
Member

@pitdicker oh good point! If that ends up being the only error then this is indeed probably good to go, I only compiled in stage0

@bors: r+ 98fa5ac

@pitdicker
Copy link
Contributor Author

They say in Dutch when you don't succeed the first two times: third time is shipping rights :). Let's hope

@bors
Copy link
Collaborator

bors commented Feb 24, 2016

⌛ Testing commit 98fa5ac with merge 321a60b...

@alexcrichton
Copy link
Member

@bors: retry force

Unfortunately this won't land until #31855 lands

@bors
Copy link
Collaborator

bors commented Feb 24, 2016

⌛ Testing commit 98fa5ac with merge df91cb9...

bors added a commit that referenced this pull request Feb 24, 2016
I am not entirely sure I have got everything right, but if it compiles it is ok probably...
I tested it with msvc x86_64 and gnu.

Somehow a lot of `EXCEPTION-*` constants are dead code when running test, no idea why.
I have put `#![cfg_attr(test, allow(dead_code))]` at the top for this.
@bors bors merged commit 98fa5ac into rust-lang:master Feb 24, 2016
@pitdicker pitdicker deleted the clean_out_windows_c branch February 25, 2016 15:18
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.

5 participants