Auto merge of rust-lang#6098 - longlb:interior_mut_const, r=ebroto

Downgrade interior_mutable_const lints to warn by default

This change updates the two lints in the file non_copy_const.rs to be warn by default rather than deny by default. It also updates the known problems for declare_interior_mutable_const to mention some issues that are affected by the lints.

This is a repeat pull request since I botched the first one (rust-lang#6012). Apart from my messing up the commits of that one, I also had a problem where the stderr of the tests didn't change despite me changing both lints to warn by default. Is this normal behaviour for some lints or do I need to adjust the tests to accommodate the change?

fixes rust-lang#5863
changelog: none

Author: bors

clippy_lints/src/lib.rs

@@ -1604,6 +1604,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&mut_reference::UNNECESSARY_MUT_PASSED),
         LintId::of(&neg_multiply::NEG_MULTIPLY),
         LintId::of(&new_without_default::NEW_WITHOUT_DEFAULT),
+        LintId::of(&non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),
+        LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
         LintId::of(&non_expressive_names::JUST_UNDERSCORES_AND_DIGITS),
         LintId::of(&non_expressive_names::MANY_SINGLE_CHAR_NAMES),
         LintId::of(&panic_unimplemented::PANIC_PARAMS),
@@ -1760,8 +1762,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&misc::FLOAT_CMP),
         LintId::of(&misc::MODULO_ONE),
         LintId::of(&mut_key::MUTABLE_KEY_TYPE),
-        LintId::of(&non_copy_const::BORROW_INTERIOR_MUTABLE_CONST),
-        LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST),
         LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS),
         LintId::of(&option_env_unwrap::OPTION_ENV_UNWRAP),
         LintId::of(&ptr::MUT_FROM_REF),

clippy_lints/src/non_copy_const.rs

@@ -1,6 +1,6 @@
 //! Checks for uses of const which the type is not `Freeze` (`Cell`-free).
 //!
-//! This lint is **deny** by default.
+//! This lint is **warn** by default.
 
 use std::ptr;
 
@@ -17,6 +17,8 @@ use rustc_typeck::hir_ty_to_ty;
 use crate::utils::{in_constant, qpath_res, span_lint_and_then};
 use if_chain::if_chain;
 
+// FIXME: this is a correctness problem but there's no suitable
+// warn-by-default category.
 declare_clippy_lint! {
     /// **What it does:** Checks for declaration of `const` items which is interior
     /// mutable (e.g., contains a `Cell`, `Mutex`, `AtomicXxxx`, etc.).
@@ -34,6 +36,15 @@ declare_clippy_lint! {
     /// `std::sync::ONCE_INIT` constant). In this case the use of `const` is legit,
     /// and this lint should be suppressed.
     ///
+    /// When an enum has variants with interior mutability, use of its non interior mutable
+    /// variants can generate false positives. See issue
+    /// [#3962](https://github.com/rust-lang/rust-clippy/issues/3962)
+    ///
+    /// Types that have underlying or potential interior mutability trigger the lint whether
+    /// the interior mutable field is used or not. See issues
+    /// [#5812](https://github.com/rust-lang/rust-clippy/issues/5812) and
+    /// [#3825](https://github.com/rust-lang/rust-clippy/issues/3825)
+    ///
     /// **Example:**
     /// ```rust
     /// use std::sync::atomic::{AtomicUsize, Ordering::SeqCst};
@@ -49,10 +60,12 @@ declare_clippy_lint! {
     /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
     /// ```
     pub DECLARE_INTERIOR_MUTABLE_CONST,
-    correctness,
+    style,
     "declaring `const` with interior mutability"
 }
 
+// FIXME: this is a correctness problem but there's no suitable
+// warn-by-default category.
 declare_clippy_lint! {
     /// **What it does:** Checks if `const` items which is interior mutable (e.g.,
     /// contains a `Cell`, `Mutex`, `AtomicXxxx`, etc.) has been borrowed directly.
@@ -64,7 +77,14 @@ declare_clippy_lint! {
     ///
     /// The `const` value should be stored inside a `static` item.
     ///
-    /// **Known problems:** None
+    /// **Known problems:** When an enum has variants with interior mutability, use of its non
+    /// interior mutable variants can generate false positives. See issue
+    /// [#3962](https://github.com/rust-lang/rust-clippy/issues/3962)
+    ///
+    /// Types that have underlying or potential interior mutability trigger the lint whether
+    /// the interior mutable field is used or not. See issues
+    /// [#5812](https://github.com/rust-lang/rust-clippy/issues/5812) and
+    /// [#3825](https://github.com/rust-lang/rust-clippy/issues/3825)
     ///
     /// **Example:**
     /// ```rust
@@ -81,7 +101,7 @@ declare_clippy_lint! {
     /// assert_eq!(STATIC_ATOM.load(SeqCst), 9); // use a `static` item to refer to the same instance
     /// ```
     pub BORROW_INTERIOR_MUTABLE_CONST,
-    correctness,
+    style,
     "referencing `const` with interior mutability"
 }
 

src/lintlist/mod.rs

@@ -110,7 +110,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
     },
     Lint {
         name: "borrow_interior_mutable_const",
-        group: "correctness",
+        group: "style",
         desc: "referencing `const` with interior mutability",
         deprecation: None,
         module: "non_copy_const",
@@ -334,7 +334,7 @@ pub static ref ALL_LINTS: Vec<Lint> = vec![
     },
     Lint {
         name: "declare_interior_mutable_const",
-        group: "correctness",
+        group: "style",
         desc: "declaring `const` with interior mutability",
         deprecation: None,
         module: "non_copy_const",