Auto merge of rust-lang#83908 - Flying-Toast:master, r=davidtwco

Add enum_intrinsics_non_enums lint

There is a clippy lint to prevent calling [`mem::discriminant`](https://doc.rust-lang.org/std/mem/fn.discriminant.html) with a non-enum type. I think the lint is worthy of being included in rustc, given that `discriminant::<T>()` where `T` is a non-enum has an unspecified return value, and there are no valid use cases where you'd actually want this.

I've also made the lint check [variant_count](https://doc.rust-lang.org/core/mem/fn.variant_count.html) (rust-lang#73662).

closes rust-lang#83899

Author: bors

compiler/rustc_lint/src/enum_intrinsics_non_enums.rs

@@ -0,0 +1,106 @@
+use crate::{context::LintContext, LateContext, LateLintPass};
+use rustc_hir as hir;
+use rustc_middle::ty::{fold::TypeFoldable, Ty};
+use rustc_span::{symbol::sym, Span};
+
+declare_lint! {
+    /// The `enum_intrinsics_non_enums` lint detects calls to
+    /// intrinsic functions that require an enum ([`core::mem::discriminant`],
+    /// [`core::mem::variant_count`]), but are called with a non-enum type.
+    ///
+    /// [`core::mem::discriminant`]: https://doc.rust-lang.org/core/mem/fn.discriminant.html
+    /// [`core::mem::variant_count`]: https://doc.rust-lang.org/core/mem/fn.variant_count.html
+    ///
+    /// ### Example
+    ///
+    /// ```rust,compile_fail
+    /// #![deny(enum_intrinsics_non_enums)]
+    /// core::mem::discriminant::<i32>(&123);
+    /// ```
+    ///
+    /// {{produces}}
+    ///
+    /// ### Explanation
+    ///
+    /// In order to accept any enum, the `mem::discriminant` and
+    /// `mem::variant_count` functions are generic over a type `T`.
+    /// This makes it technically possible for `T` to be a non-enum,
+    /// in which case the return value is unspecified.
+    ///
+    /// This lint prevents such incorrect usage of these functions.
+    ENUM_INTRINSICS_NON_ENUMS,
+    Deny,
+    "detects calls to `core::mem::discriminant` and `core::mem::variant_count` with non-enum types"
+}
+
+declare_lint_pass!(EnumIntrinsicsNonEnums => [ENUM_INTRINSICS_NON_ENUMS]);
+
+/// Returns `true` if we know for sure that the given type is not an enum. Note that for cases where
+/// the type is generic, we can't be certain if it will be an enum so we have to assume that it is.
+fn is_non_enum(t: Ty<'_>) -> bool {
+    !t.is_enum() && !t.potentially_needs_subst()
+}
+
+fn enforce_mem_discriminant(
+    cx: &LateContext<'_>,
+    func_expr: &hir::Expr<'_>,
+    expr_span: Span,
+    args_span: Span,
+) {
+    let ty_param = cx.typeck_results().node_substs(func_expr.hir_id).type_at(0);
+    if is_non_enum(ty_param) {
+        cx.struct_span_lint(ENUM_INTRINSICS_NON_ENUMS, expr_span, |builder| {
+            builder
+                .build(
+                    "the return value of `mem::discriminant` is \
+                        unspecified when called with a non-enum type",
+                )
+                .span_note(
+                    args_span,
+                    &format!(
+                        "the argument to `discriminant` should be a \
+                            reference to an enum, but it was passed \
+                            a reference to a `{}`, which is not an enum.",
+                        ty_param,
+                    ),
+                )
+                .emit();
+        });
+    }
+}
+
+fn enforce_mem_variant_count(cx: &LateContext<'_>, func_expr: &hir::Expr<'_>, span: Span) {
+    let ty_param = cx.typeck_results().node_substs(func_expr.hir_id).type_at(0);
+    if is_non_enum(ty_param) {
+        cx.struct_span_lint(ENUM_INTRINSICS_NON_ENUMS, span, |builder| {
+            builder
+                .build(
+                    "the return value of `mem::variant_count` is \
+                        unspecified when called with a non-enum type",
+                )
+                .note(&format!(
+                    "the type parameter of `variant_count` should \
+                            be an enum, but it was instantiated with \
+                            the type `{}`, which is not an enum.",
+                    ty_param,
+                ))
+                .emit();
+        });
+    }
+}
+
+impl<'tcx> LateLintPass<'tcx> for EnumIntrinsicsNonEnums {
+    fn check_expr(&mut self, cx: &LateContext<'_>, expr: &hir::Expr<'_>) {
+        if let hir::ExprKind::Call(ref func, ref args) = expr.kind {
+            if let hir::ExprKind::Path(ref qpath) = func.kind {
+                if let Some(def_id) = cx.qpath_res(qpath, func.hir_id).opt_def_id() {
+                    if cx.tcx.is_diagnostic_item(sym::mem_discriminant, def_id) {
+                        enforce_mem_discriminant(cx, func, expr.span, args[0].span);
+                    } else if cx.tcx.is_diagnostic_item(sym::mem_variant_count, def_id) {
+                        enforce_mem_variant_count(cx, func, expr.span);
+                    }
+                }
+            }
+        }
+    }
+}

compiler/rustc_lint/src/lib.rs

@@ -47,6 +47,7 @@ mod array_into_iter;
 pub mod builtin;
 mod context;
 mod early;
+mod enum_intrinsics_non_enums;
 mod internal;
 mod late;
 mod levels;
@@ -76,6 +77,7 @@ use rustc_span::Span;
 
 use array_into_iter::ArrayIntoIter;
 use builtin::*;
+use enum_intrinsics_non_enums::EnumIntrinsicsNonEnums;
 use internal::*;
 use methods::*;
 use non_ascii_idents::*;
@@ -168,6 +170,7 @@ macro_rules! late_lint_passes {
                 TemporaryCStringAsPtr: TemporaryCStringAsPtr,
                 NonPanicFmt: NonPanicFmt,
                 NoopMethodCall: NoopMethodCall,
+                EnumIntrinsicsNonEnums: EnumIntrinsicsNonEnums,
                 InvalidAtomicOrdering: InvalidAtomicOrdering,
                 NamedAsmLabels: NamedAsmLabels,
             ]

compiler/rustc_span/src/symbol.rs

@@ -816,6 +816,7 @@ symbols! {
         mem_size_of,
         mem_size_of_val,
         mem_uninitialized,
+        mem_variant_count,
         mem_zeroed,
         member_constraints,
         memory,

library/core/src/mem/mod.rs

@@ -1053,6 +1053,7 @@ pub const fn discriminant<T>(v: &T) -> Discriminant<T> {
 #[inline(always)]
 #[unstable(feature = "variant_count", issue = "73662")]
 #[rustc_const_unstable(feature = "variant_count", issue = "73662")]
+#[rustc_diagnostic_item = "mem_variant_count"]
 pub const fn variant_count<T>() -> usize {
     intrinsics::variant_count::<T>()
 }

src/test/ui/consts/const-variant-count.rs

@@ -1,5 +1,5 @@
 // run-pass
-#![allow(dead_code)]
+#![allow(dead_code, enum_intrinsics_non_enums)]
 #![feature(variant_count)]
 #![feature(never_type)]
 

src/test/ui/enum-discriminant/discriminant_value-wrapper.rs

@@ -1,4 +1,7 @@
 // run-pass
+
+#![allow(enum_intrinsics_non_enums)]
+
 use std::mem;
 
 enum ADT {

src/test/ui/lint/lint-enum-intrinsics-non-enums.rs

@@ -0,0 +1,67 @@
+// Test the enum_intrinsics_non_enums lint.
+
+#![feature(variant_count)]
+
+use std::mem::{discriminant, variant_count};
+
+enum SomeEnum {
+    A,
+    B,
+}
+
+struct SomeStruct;
+
+fn generic_discriminant<T>(v: &T) {
+    discriminant::<T>(v);
+}
+
+fn generic_variant_count<T>() -> usize {
+    variant_count::<T>()
+}
+
+fn test_discriminant() {
+    discriminant(&SomeEnum::A);
+    generic_discriminant(&SomeEnum::B);
+
+    discriminant(&());
+    //~^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+
+    discriminant(&&SomeEnum::B);
+    //~^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+
+    discriminant(&SomeStruct);
+    //~^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+
+    discriminant(&123u32);
+    //~^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+
+    discriminant(&&123i8);
+    //~^ error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+}
+
+fn test_variant_count() {
+    variant_count::<SomeEnum>();
+    generic_variant_count::<SomeEnum>();
+
+    variant_count::<&str>();
+    //~^ error: the return value of `mem::variant_count` is unspecified when called with a non-enum type
+
+    variant_count::<*const u8>();
+    //~^ error: the return value of `mem::variant_count` is unspecified when called with a non-enum type
+
+    variant_count::<()>();
+    //~^ error: the return value of `mem::variant_count` is unspecified when called with a non-enum type
+
+    variant_count::<&SomeEnum>();
+    //~^ error: the return value of `mem::variant_count` is unspecified when called with a non-enum type
+}
+
+fn main() {
+    test_discriminant();
+    test_variant_count();
+
+    // The lint ignores cases where the type is generic, so these should be
+    // allowed even though their return values are unspecified
+    generic_variant_count::<SomeStruct>();
+    generic_discriminant::<SomeStruct>(&SomeStruct);
+}

src/test/ui/lint/lint-enum-intrinsics-non-enums.stderr

@@ -0,0 +1,95 @@
+error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:26:5
+   |
+LL |     discriminant(&());
+   |     ^^^^^^^^^^^^^^^^^
+   |
+   = note: `#[deny(enum_intrinsics_non_enums)]` on by default
+note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `()`, which is not an enum.
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:26:18
+   |
+LL |     discriminant(&());
+   |                  ^^^
+
+error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:29:5
+   |
+LL |     discriminant(&&SomeEnum::B);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `&SomeEnum`, which is not an enum.
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:29:18
+   |
+LL |     discriminant(&&SomeEnum::B);
+   |                  ^^^^^^^^^^^^^
+
+error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:32:5
+   |
+LL |     discriminant(&SomeStruct);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `SomeStruct`, which is not an enum.
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:32:18
+   |
+LL |     discriminant(&SomeStruct);
+   |                  ^^^^^^^^^^^
+
+error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:35:5
+   |
+LL |     discriminant(&123u32);
+   |     ^^^^^^^^^^^^^^^^^^^^^
+   |
+note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `u32`, which is not an enum.
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:35:18
+   |
+LL |     discriminant(&123u32);
+   |                  ^^^^^^^
+
+error: the return value of `mem::discriminant` is unspecified when called with a non-enum type
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:38:5
+   |
+LL |     discriminant(&&123i8);
+   |     ^^^^^^^^^^^^^^^^^^^^^
+   |
+note: the argument to `discriminant` should be a reference to an enum, but it was passed a reference to a `&i8`, which is not an enum.
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:38:18
+   |
+LL |     discriminant(&&123i8);
+   |                  ^^^^^^^
+
+error: the return value of `mem::variant_count` is unspecified when called with a non-enum type
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:46:5
+   |
+LL |     variant_count::<&str>();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the type parameter of `variant_count` should be an enum, but it was instantiated with the type `&str`, which is not an enum.
+
+error: the return value of `mem::variant_count` is unspecified when called with a non-enum type
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:49:5
+   |
+LL |     variant_count::<*const u8>();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the type parameter of `variant_count` should be an enum, but it was instantiated with the type `*const u8`, which is not an enum.
+
+error: the return value of `mem::variant_count` is unspecified when called with a non-enum type
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:52:5
+   |
+LL |     variant_count::<()>();
+   |     ^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the type parameter of `variant_count` should be an enum, but it was instantiated with the type `()`, which is not an enum.
+
+error: the return value of `mem::variant_count` is unspecified when called with a non-enum type
+  --> $DIR/lint-enum-intrinsics-non-enums.rs:55:5
+   |
+LL |     variant_count::<&SomeEnum>();
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: the type parameter of `variant_count` should be an enum, but it was instantiated with the type `&SomeEnum`, which is not an enum.
+
+error: aborting due to 9 previous errors
+

src/tools/clippy/CHANGELOG.md

@@ -1873,10 +1873,10 @@ Released 2019-01-17
 
 [2e26fdc2...b2601be](https://github.com/rust-lang/rust-clippy/compare/2e26fdc2...b2601be)
 
-* New lints: [`slow_vector_initialization`], [`mem_discriminant_non_enum`],
+* New lints: [`slow_vector_initialization`], `mem_discriminant_non_enum`,
   [`redundant_clone`], [`wildcard_dependencies`],
   [`into_iter_on_ref`], `into_iter_on_array`, [`deprecated_cfg_attr`],
-  [`mem_discriminant_non_enum`], [`cargo_common_metadata`]
+  [`cargo_common_metadata`]
 * Add support for `u128` and `i128` to integer related lints
 * Add float support to `mistyped_literal_suffixes`
 * Fix false positives in `use_self`
@@ -2839,7 +2839,6 @@ Released 2018-09-13
 [`match_wild_err_arm`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wild_err_arm
 [`match_wildcard_for_single_variants`]: https://rust-lang.github.io/rust-clippy/master/index.html#match_wildcard_for_single_variants
 [`maybe_infinite_iter`]: https://rust-lang.github.io/rust-clippy/master/index.html#maybe_infinite_iter
-[`mem_discriminant_non_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_discriminant_non_enum
 [`mem_forget`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_forget
 [`mem_replace_option_with_none`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_option_with_none
 [`mem_replace_with_default`]: https://rust-lang.github.io/rust-clippy/master/index.html#mem_replace_with_default

src/tools/clippy/clippy_lints/src/lib.register_all.rs

@@ -127,7 +127,6 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(matches::REDUNDANT_PATTERN_MATCHING),
     LintId::of(matches::SINGLE_MATCH),
     LintId::of(matches::WILDCARD_IN_OR_PATTERNS),
-    LintId::of(mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
     LintId::of(mem_replace::MEM_REPLACE_OPTION_WITH_NONE),
     LintId::of(mem_replace::MEM_REPLACE_WITH_DEFAULT),
     LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT),

src/tools/clippy/clippy_lints/src/lib.register_correctness.rs

@@ -36,7 +36,6 @@ store.register_group(true, "clippy::correctness", Some("clippy_correctness"), ve
     LintId::of(loops::ITER_NEXT_LOOP),
     LintId::of(loops::NEVER_LOOP),
     LintId::of(loops::WHILE_IMMUTABLE_CONDITION),
-    LintId::of(mem_discriminant::MEM_DISCRIMINANT_NON_ENUM),
     LintId::of(mem_replace::MEM_REPLACE_WITH_UNINIT),
     LintId::of(methods::CLONE_DOUBLE_REF),
     LintId::of(methods::ITERATOR_STEP_BY_ZERO),

src/tools/clippy/clippy_lints/src/lib.register_lints.rs

@@ -241,7 +241,6 @@ store.register_lints(&[
     matches::SINGLE_MATCH_ELSE,
     matches::WILDCARD_ENUM_MATCH_ARM,
     matches::WILDCARD_IN_OR_PATTERNS,
-    mem_discriminant::MEM_DISCRIMINANT_NON_ENUM,
     mem_forget::MEM_FORGET,
     mem_replace::MEM_REPLACE_OPTION_WITH_NONE,
     mem_replace::MEM_REPLACE_WITH_DEFAULT,

src/tools/clippy/clippy_lints/src/lib.rs

@@ -266,7 +266,6 @@ mod map_unit_fn;
 mod match_on_vec_items;
 mod match_result_ok;
 mod matches;
-mod mem_discriminant;
 mod mem_forget;
 mod mem_replace;
 mod methods;
@@ -600,7 +599,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
     let doc_valid_idents = conf.doc_valid_idents.iter().cloned().collect::<FxHashSet<_>>();
     store.register_late_pass(move || Box::new(doc::DocMarkdown::new(doc_valid_idents.clone())));
     store.register_late_pass(|| Box::new(neg_multiply::NegMultiply));
-    store.register_late_pass(|| Box::new(mem_discriminant::MemDiscriminant));
     store.register_late_pass(|| Box::new(mem_forget::MemForget));
     store.register_late_pass(|| Box::new(arithmetic::Arithmetic::default()));
     store.register_late_pass(|| Box::new(assign_ops::AssignOps));
@@ -850,6 +848,7 @@ pub fn register_renamed(ls: &mut rustc_lint::LintStore) {
     ls.register_renamed("clippy::panic_params", "non_fmt_panics");
     ls.register_renamed("clippy::unknown_clippy_lints", "unknown_lints");
     ls.register_renamed("clippy::invalid_atomic_ordering", "invalid_atomic_ordering");
+    ls.register_renamed("clippy::mem_discriminant_non_enum", "enum_intrinsics_non_enums");
 }
 
 // only exists to let the dogfood integration test works.