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

Tracking issue for `resolve_trait_on_defaulted_unit` compatibility lint #39216

Open
canndrew opened this Issue Jan 21, 2017 · 10 comments

Comments

Projects
None yet
8 participants
@canndrew
Contributor

canndrew commented Jan 21, 2017

This is the summary issue for the resolve_trait_on_defaulted_unit future-compatibility warning and other related errors. The goal of this page is describe why this change was made and how you can fix code that is affected by it. It also provides a place to ask questions or register a complaint if you feel the change should not be made. For more information on the policy around future-compatibility warnings, see our breaking change policy guidelines.

What is this lint about

Ordinarily, when a user doesn't specify the type of an expression and the type cannot be inferred, rust will raise an error. For example, consider this code:

let _ = Default::default();

Because we haven't specified the type of _ the compiler doesn't know what type of value the user is asking for a default of. And so we get this error:

error[E0282]: unable to infer enough type information about `Self`
 --> example.rs:2:13
  |
2 |     let _ = Default::default();
  |             ^^^^^^^^^^^^^^^^ cannot infer type for `Self`
  |
  = note: type annotations or generic parameter binding required

error: aborting due to previous error

However, due to an unfortunate quirk in Rust's type inference algorithms it is sometimes possible to sneak situations like this past the compiler. In these cases, the unspecified type is defaulted to (). For an example of this, you can try deserializing an unspecified type with serde:

let _ = Deserialize::deserialize(foo)?;

In this case, the code will compile and will deserialize a value of type ().

This behaviour is set to change with the eventual rolling-out of feature(never_type). Where defaulting is still used types will instead default to !. In the best case, code that currently relies on unspecified types defaulting to () will stop compiling. In the worst case, code will continue to compile but may execute differently, as in the above example where a ! will be deserialised instead.

The resolve_trait_on_defaulted_unit warning is raised wherever the compiler thinks your program's behaviour may depend on the current defaulting rules.

How to fix this warning/error

Be specific about what type you're using. In the serde example above this could be done by simply adding a type annotation:

let _: () = Deserialize::deserialize(foo)?;

Current status

  • #39009 introduces the resolve_trait_on_defaulted_unit lint as warn-by-default
  • #42894 makes the resolve_trait_on_defaulted_unit lint deny-by-default
  • PR ? makes the resolve_trait_on_defaulted_unit lint a hard error
@dtolnay

This comment has been minimized.

Member

dtolnay commented Feb 16, 2017

Is there any hope of treating let _ = Deserialize::deserialize(foo)? more like let _ = Default::default() and producing the same "cannot infer type" message? Inferring ! does not make any more or less sense to me than inferring ().

Has this already been discussed elsewhere? I don't see any links in your comment.

@alexcrichton

This comment has been minimized.

Member

alexcrichton commented Feb 16, 2017

FWIW futures-rs ran into this warning and I found it pretty hard to fix, I was just randomly annotating unknown types until I finally found that location in the commit.

Also out of curiosity, what PR was this introduced with?

@canndrew

This comment has been minimized.

Contributor

canndrew commented Feb 21, 2017

@dtolnay: @eddyb and @nikomatsakis have brought that idea up before and they might have an opinion on whether it's worth trying to implement.

@alexcrichton: This was added in #39009. I guess the diagnostics will need to be improved :/

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Feb 21, 2017

@dtolnay I believe that case of the ? expressions in particular was a bug, and should have been fixed by #39485.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Feb 21, 2017

@alexcrichton hmm, yeah, that's frustrating; we're not super good at highlighting where a type annotation is needed, though we've been getting somewhat better. Maybe "closure return type" would make a good example.

@canndrew

This comment has been minimized.

Contributor

canndrew commented Feb 27, 2017

@dtolnay I believe that case of the ? expressions in particular was a bug, and should have been fixed by #39485.

True, though the general case of us having "defaulting" behavior around ()/! at all still remains.

@brson brson added the B-unstable label Mar 1, 2017

@bluss

This comment has been minimized.

Contributor

bluss commented Mar 6, 2017

Thanks for the extensive write-up. petgraph's test suite triggered this in:

warning: code relies on type inference rules which are likely to change
   --> src/algo/dominators.rs:260:9
    |
260 |         assert_eq!(None, doms.dominators(99).map(|_| unreachable!()));
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
    = note: #[warn(resolve_trait_on_defaulted_unit)] on by default
    = warning: this was previously accepted by the compiler but is being phased out; it will become a hard error in a future release!
    = note: for more information, see issue #39216 <https://github.com/rust-lang/rust/issues/39216>
    = note: this error originates in a macro outside of the current crate

Solution is to use for example None::<()> to have the type explicit. We'd of course like to use None::<!> when stable.

petgraph might have the record in running into most Rust breaking changes ever.

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Mar 6, 2017

@bluss note that i've overhauling a lot of this machinery to address regressions in #40224 as well. I'm not sure what affect this will have on petgraph though.

@Xion

This comment has been minimized.

Xion commented Jan 22, 2018

I've got to this issue from a compiler message when trying some serde/serde_json workaround, and I noticed the message still says the problem will become a hard error.

Well, it is a hard error already :) I'm thinking the message should be reworded perhaps?

@nikomatsakis

This comment has been minimized.

Contributor

nikomatsakis commented Jan 23, 2018

@Xion it is not, but we are starting to move towards making it so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment