taiki-e/pin-project-lite

pin-project-lite 0.1.9 breaks surf v2.0.0-alpha.*.

xu-cheng opened this issue · 6 comments

xu-cheng commented

To reproduce the problem, create a new cargo project with the following dependencies:

[dependencies]
pin-project-lite = "0.1.9"
surf = "2.0.0-alpha.6"

Run cargo check. It will report the following error:

    Checking surf v2.0.0-alpha.6
error[E0453]: allow(explicit_outlives_requirements) overruled by outer forbid(rust_2018_idioms)
  --> /Users/chengxu/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/surf-2.0.0-alpha.6/src/response.rs:17:1
   |
17 | / pin_project_lite::pin_project! {
18 | |     /// An HTTP response, returned by `Request`.
19 | |     pub struct Response {
20 | |         #[pin]
21 | |         res: http_client::Response,
22 | |     }
23 | | }
   | |_^ overruled by previous forbid
   |
  ::: /Users/chengxu/.local/share/cargo/registry/src/github.com-1ecc6299db9ec823/surf-2.0.0-alpha.6/src/lib.rs:73:11
   |
73 |   #![forbid(rust_2018_idioms)]
   |             ---------------- `forbid` level set here
   |
   = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to previous error

For more information about this error, try `rustc --explain E0453`.
error: could not compile `surf`.

To learn more, run the command again with --verbose.
taiki-e commented

forbid is opt-in and basically means "if rustc adds a new warning or an external macro allows lint, a previously released version may not compile". (forbid(unsafe_code) has another meaning)

And basically, most macros do not guarantee compatibility with forbid, as lints warn of accidentally (or intentionally) macro-generated code. (In pin-project/pin-project-lite, the only exception is forbid(unsafe_code), because compatibility with forbid(unsafe_code) has another meaning.)

So, as long as there is good reason to allow that lint, I believe this is a problem on the part of the user using forbid.

FYI: allow(explicit_outlives_requirements) (added in 0.1.9) is needed to silence rustc's warning on macro-generated code. (Without this, you would have to write #![allow(...)] at the module level to allow explicit_outlives_requirements.)

taiki-e commented

Closing in favor of http-rs/surf#244.

taiki-e commented

This will be fixed in http-rs/surf#245

  • ❤️1
yoshuawuyts commented

forbid is opt-in and basically means "if rustc adds a new warning or an external macro allows lint, a previously released version may not compile"

The "rust 2018" idioms lint should only be cover the rules introduced for the 2018 edition; the plan as I understood it was that during the 2018 edition these opt-in lints would gradually become warnings out of the box. And in the next edition these will become hard errors and disallowed.

Reality is a bit more fluid, and it's unclear what exactly will happen in the 2021 edition. But unless I'm mistaken, the set of rules covered by rust_2018_idioms was pretty much defined at the start of the 2018 edition, and we don't need to worry about more lints being added to it.

taiki-e commented

But unless I'm mistaken, the set of rules covered by rust_2018_idioms was pretty much defined at the start of the 2018 edition, and we don't need to worry about more lints being added to it.

"Add a new warning" does not mean "Add a new lint". It includes adding warnings to existing lint.

For example:

  • explicit_outlives_requirements had a large number of false positives / false negatives at the 1.31, which were significantly improved at 1.37 (rust-lang/rust#61172).
  • unused_imports did not previously warn redundant imports, but now it can. (rust-lang/rust#58805)

Both are "improvements to existing lint", but they are also "adds warnings". So crates released with forbid/deny prior to these improvements might not compile with rustc version that contains these improvements.

And in the next edition these will become hard errors and disallowed.

If I remember correctly, this should only happen in parts of rust_2018_idioms, not all.

  • bare-trait-objects, ellipsis-inclusive-range-patterns, and elided-lifetimes-in-paths are warnings about deprecated syntax and may be hard errors in the future editions. (e.g., elided-lifetimes-in-paths is already hard errors on async functions)
  • On the other hand, unused-extern-crates and explicit-outlives-requirements belong to almost the same category as unused, so they do not need to be hard errors.
yoshuawuyts commented

So crates released with forbid/deny prior to these improvements might not compile with rustc version that contains these improvements.

Ah yeah, that's very fair. I think you're right that these lints should have been warn all along; I was wrong in my assumptions around them.