Skip to content

Spurious warnings around uninhabited types with 1.33 #58844

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
nox opened this issue Mar 1, 2019 · 5 comments
Closed

Spurious warnings around uninhabited types with 1.33 #58844

nox opened this issue Mar 1, 2019 · 5 comments

Comments

@nox
Copy link
Contributor

nox commented Mar 1, 2019

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=25adf77e74f612769891852bfeaf237c

Using uninhabited enums for some targets or combinations of crate features are a useful common idiom and this probably shouldn't emit any warning.

@varkor
Copy link
Member

varkor commented Mar 2, 2019

What problem are you seeing here? That let var = /* diverging expression */; marks var as an unused variable? Could you give an example where this isn't reasonable behaviour (i.e. can't trivially be refactored to be avoided)?

@nox
Copy link
Contributor Author

nox commented Mar 2, 2019

What do you mean by trivially? This is a convenient way to represent platform-specific behaviour: have a type an uninhabited enum on some platforms and something useful in others, and then you can write code that abstracts over the two.

This is what the code is in that case:

https://github.com/servo/webrender/blob/89c79c227245e6fb7c447de157058e65b7868189/wrench/src/angle.rs#L10-L73

I also use uninhabited enums extensively in serde_urlencoded:

https://github.com/nox/serde_urlencoded/blob/3ae533293191ee8339b92ee51faee1fdd5900002/src/ser/part.rs#L39-L45

Actually serde defines the uninhabited type itself:

https://github.com/serde-rs/serde/blob/master/serde/src/ser/impossible.rs

I don't see why an empty enum should trigger the warning, we literally have an issue filed against this very repo about not making #[derive(Debug)] warn on an empty enum for example, why is this any different?

@nox
Copy link
Contributor Author

nox commented Mar 2, 2019

Upstream is going to prefix both concerned variables in webrender with an _, even though those values are indeed used on Windows where Context isn't an empty enum.

Not knowing about the fact that empty enums can lead to warnings, I would be myself really confused as to why the variables are prefixed, given they totally look used in the code, even if only on some platforms.

This new spurious warning means you basically have to either prefix variables with _ (confusing), mark every snippet that may use such uninhabited types with #[allow(…)] (also confusing, why is the allow there when all the code looks used and there is no apparently unreachable code), or mark every snippet with #[cfg(…, allow(…))] (tiresome because I don't remember at all instants on which platforms the thing is uninhabited). Meanwhile, if there is any really unused variable in the code, those legit warnings will be silenced.

@nikic
Copy link
Contributor

nikic commented Mar 2, 2019

@nox Inhabitedness checks are privacy based, so consuming code will only get these warnings if it "knows" that the type is uninhabited. Maybe you can use something like pub struct Context { privately_uninhabited: ! } to solve this? (Or an empty enum for the !, the important part is that it's not public.)

@nox
Copy link
Contributor Author

nox commented Mar 2, 2019

That is helpful, thanks, I wasn't aware of how visibility affects the inhabitedness check. Closing this issue.

@nox nox closed this as completed Mar 2, 2019
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

No branches or pull requests

3 participants