rust-lang/rust

beta regression: forbid(warnings) interacts poorly with common derives.

Closed this issue · 4 comments

Spawned off of #80988, but specifically talking about forbid and derive interacting in a surprising manner.

Code

I tried this code (play):

#![forbid(warnings)]

#[derive(serde::Serialize)]
struct Bar;

I expected to see this happen: Successful diagnostic-free compile, just like what happens on stable 1.49 Rust

Instead, this happened:

error[E0453]: allow(non_upper_case_globals) incompatible with previous forbid
 --> src/lib.rs:3:10
  |
1 | #![forbid(warnings)]
  |           -------- `forbid` level set here
2 | 
3 | #[derive(serde::Serialize)]
  |          ^^^^^^^^^^^^^^^^ overruled by previous forbid
  |
  = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0453]: allow(unused_attributes) incompatible with previous forbid
 --> src/lib.rs:3:10
  |
1 | #![forbid(warnings)]
  |           -------- `forbid` level set here
2 | 
3 | #[derive(serde::Serialize)]
  |          ^^^^^^^^^^^^^^^^ overruled by previous forbid
  |
  = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0453]: allow(rust_2018_idioms) incompatible with previous forbid
 --> src/lib.rs:3:10
  |
1 | #![forbid(warnings)]
  |           -------- `forbid` level set here
2 | 
3 | #[derive(serde::Serialize)]
  |          ^^^^^^^^^^^^^^^^ overruled by previous forbid
  |
  = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0453]: allow(unused_macros) incompatible with previous forbid
 --> src/lib.rs:3:10
  |
1 | #![forbid(warnings)]
  |           -------- `forbid` level set here
2 | 
3 | #[derive(serde::Serialize)]
  |          ^^^^^^^^^^^^^^^^ overruled by previous forbid
  |
  = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 4 previous errors

Version it worked on

It most recently worked on: Rust 1.49

Version with regression

1.50.0-beta.6

@rustbot modify labels: +regression-from-stable-to-beta -regression-untriaged

Assigning P-high as sibling issue #80988

This is a regression because previously the "lowering forbid" lint did not actually detect lint groups as being forbidden, i.e., the allows within the derive or elsewhere would be silently accepted and used. Derive was not previously (or currently) treated in any special way for the forbid handling specifically.

This issue intends for the lang team to decide what to do with this. We have a number of options:

  1. accept that people should not use forbid with wide-sweeping lints (e.g., warnings) as this may break proc macros. targeted lints are more likely to be ok, but still may break when proc macros add allow. (The breakage, in any case, will be strictly local as cap-lints applies to the forbid checking; fixing it will be a matter of replacing forbid with deny in the easy case).
  2. try to assemble a list of lints which are commonly allowed within derives or other macros, and silence those within macro-expanded code such that allow is not necessary; this would let internal lints and the ecosystem stop issuing allows, in theory (though backwards compat for crates with longer version support cycles may be interesting here)
  3. try to formulate some kind of hygiene, where forbid does not "reach into" macros ever.
  4. try to rework derives such that the lints currently allowed are not emitted because of different code being expanded. It seems likely this will not be plausible long-term.

There may be other options, but these seem like the most apparent. For all but option 1 we will likely want to revert or apply targeted patches to avoid breakage in the short term while we figure out a proper solution.

(note: I will not be able to attend lang triage this week, but am happy to answer questions async here or on zulip)

A fourth possibility would be if derive could somehow be fixable to not need to allow that lint.

Fix backported to 1.50.0 and landed in 1.51.0, closing this.