Skip to content

core/std: squash dead_code warnings from fail! invocations. #16196

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

Closed
wants to merge 2 commits into from

Conversation

huonw
Copy link
Member

@huonw huonw commented Aug 2, 2014

The fail macro defines some function/static items internally, which got
a dead_code warning when fail!() is used inside a dead function. This
is ugly and unnecessarily reveals implementation details, so the
warnings can be squashed.

Fixes #16192.

@sfackler
Copy link
Member

sfackler commented Aug 2, 2014

Would this cause breakage if someone has a #[forbid(dead_code)]?

@lilyball
Copy link
Contributor

lilyball commented Aug 2, 2014

It probably would.

The functions/statics could just be renamed to start with _ instead.

@huonw
Copy link
Member Author

huonw commented Aug 3, 2014

Theoretically it wouldn't cause actual breakage, because the only way for this to pop up is when inside a dead function (i.e. that would be flagged as an error anyway), but it does cause noise in that case, so I changed it. NB. I had to adjust dead_code to ignore items with a leading _.

(The core/std: ... commit is the HEAD.)

@lilyball
Copy link
Contributor

lilyball commented Aug 4, 2014

@huonw I'm actually surprised the dead_code warning doesn't just suppress warnings about items that are inside items that are considered dead_code already. Or at least, consider any enclosing items to be live for the purposes of calculating if a given item is dead.

bors added a commit that referenced this pull request Aug 7, 2014
The fail macro defines some function/static items internally, which got
a dead_code warning when `fail!()` is used inside a dead function. This
is ugly and unnecessarily reveals implementation details, so the
warnings can be squashed.

Fixes #16192.
@huonw
Copy link
Member Author

huonw commented Aug 7, 2014

Was this cancelled by someone, or are the exceptions just spurious?

@lilyball
Copy link
Contributor

lilyball commented Aug 7, 2014

I don't know why it's not listed but auto-mac-64-opt failed: http://buildbot.rust-lang.org/builders/auto-mac-64-opt/builds/724/

@huonw
Copy link
Member Author

huonw commented Aug 8, 2014

Does someone have an inkling about what is going on here? It is failing with a pile of errors like

    <std macros>:7:9: 7:69 error: code is never used: `FILE_LINE`
    <std macros>:7         static FILE_LINE: (&'static str, uint) = (file!(), line!());
                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

but git grep FILE_LINE turns up:

src/libcore/macros.rs:41:            static _FILE_LINE: (&'static str, uint) = (file!(), line!());
src/libcore/macros.rs:42:            ::core::failure::begin_unwind(fmt, &_FILE_LINE)
src/libstd/macros.rs:46:        static _FILE_LINE: (&'static str, uint) = (file!(), line!());
src/libstd/macros.rs:47:        let (file, line) = _FILE_LINE;
src/libstd/macros.rs:69:            static _FILE_LINE: (&'static str, uint) = (file!(), line!());
src/libstd/macros.rs:70:            ::std::rt::begin_unwind_fmt(fmt, &_FILE_LINE)

i.e. the thing it's complaining about doesn't seem to exist.

(It passes locally.)

@lilyball
Copy link
Contributor

lilyball commented Aug 8, 2014

That's pretty strange. Sure makes it look like it's using the old macros.

@alexcrichton
Copy link
Member

When you did a git grep, are you sure you did a grep on the merged tree rather than the state of your PR as-is?

This generalises the behaviour with struct fields (which recieve no
dead_code warning if they have a leading _), and other similar lints, to
all items, e.g. `fn _foo() {} fn main() {}` has no warnings.
@huonw
Copy link
Member Author

huonw commented Aug 10, 2014

Oh, I didn't even think of that...

@lilyball
Copy link
Contributor

fn main() needs to be pub (in run-pass/dead-code-leading-underscore.rs), because of the pretty tests.

The fail macro defines some function/static items internally, which got
a dead_code warning when `fail!()` is used inside a dead function. This
is ugly and unnecessarily reveals implementation details, so the
warnings can be squashed.

Fixes rust-lang#16192.
bors added a commit that referenced this pull request Aug 11, 2014
The fail macro defines some function/static items internally, which got
a dead_code warning when `fail!()` is used inside a dead function. This
is ugly and unnecessarily reveals implementation details, so the
warnings can be squashed.

Fixes #16192.
@bors bors closed this Aug 11, 2014
@huonw huonw deleted the fail-dead-code branch December 4, 2014 02:04
@huonw huonw restored the fail-dead-code branch December 4, 2014 02:04
@huonw huonw deleted the fail-dead-code branch December 4, 2014 02:04
bors added a commit to rust-lang-ci/rust that referenced this pull request Jan 21, 2024
…e, r=Veykril

Rename generator to coroutine

Follow the rename in nightly (see https://blog.rust-lang.org/inside-rust/2023/10/23/coroutines.html)

This makes it much easier to test code with the nightly compiler.
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.

Dead code warning in fail!() expansion
5 participants