taiki-e / pin-project-lite

A lightweight version of pin-project written with declarative macros.
https://docs.rs/pin-project-lite
Apache License 2.0
216 stars 15 forks source link

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

Closed xu-cheng closed 3 years ago

xu-cheng commented 3 years ago

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 3 years ago

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 3 years ago

Closing in favor of https://github.com/http-rs/surf/issues/244.

taiki-e commented 3 years ago

This will be fixed in https://github.com/http-rs/surf/pull/245

yoshuawuyts commented 3 years ago

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 3 years ago

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:

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.

yoshuawuyts commented 3 years ago

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.