Auto merge of #34755 - jonas-schievink:minor-differences, r=eddyb

Move variant_size_differences out of trans

Also enhances the error message a bit, fixes #30505 on the way, and adds
a test (which was missing).

Closes #34018

Author: bors

src/librustc/lint/builtin.rs

@@ -94,12 +94,6 @@ declare_lint! {
     "unknown crate type found in #[crate_type] directive"
 }
 
-declare_lint! {
-    pub VARIANT_SIZE_DIFFERENCES,
-    Allow,
-    "detects enums with widely varying variant sizes"
-}
-
 declare_lint! {
     pub FAT_PTR_TRANSMUTES,
     Allow,
@@ -230,7 +224,6 @@ impl LintPass for HardwiredLints {
             UNUSED_FEATURES,
             STABLE_FEATURES,
             UNKNOWN_CRATE_TYPES,
-            VARIANT_SIZE_DIFFERENCES,
             FAT_PTR_TRANSMUTES,
             TRIVIAL_CASTS,
             TRIVIAL_NUMERIC_CASTS,

src/librustc/lint/context.rs

@@ -29,13 +29,12 @@ use dep_graph::DepNode;
 use middle::privacy::AccessLevels;
 use ty::TyCtxt;
 use session::{config, early_error, Session};
-use lint::{Level, LevelSource, Lint, LintId, LintArray, LintPass};
-use lint::{EarlyLintPassObject, LateLintPass, LateLintPassObject};
+use lint::{Level, LevelSource, Lint, LintId, LintPass};
+use lint::{EarlyLintPassObject, LateLintPassObject};
 use lint::{Default, CommandLine, Node, Allow, Warn, Deny, Forbid};
 use lint::builtin;
 use util::nodemap::FnvHashMap;
 
-use std::cell::RefCell;
 use std::cmp;
 use std::default::Default as StdDefault;
 use std::mem;
@@ -311,10 +310,6 @@ pub struct LateContext<'a, 'tcx: 'a> {
     /// levels, this stack keeps track of the previous lint levels of whatever
     /// was modified.
     level_stack: Vec<(LintId, LevelSource)>,
-
-    /// Level of lints for certain NodeIds, stored here because the body of
-    /// the lint needs to run in trans.
-    node_levels: RefCell<FnvHashMap<(ast::NodeId, LintId), LevelSource>>,
 }
 
 /// Context for lint checking of the AST, after expansion, before lowering to
@@ -664,7 +659,6 @@ impl<'a, 'tcx> LateContext<'a, 'tcx> {
             access_levels: access_levels,
             lints: lint_store,
             level_stack: vec![],
-            node_levels: RefCell::new(FnvHashMap()),
         }
     }
 
@@ -1064,38 +1058,6 @@ impl<'a, 'tcx> IdVisitingOperation for LateContext<'a, 'tcx> {
     }
 }
 
-// This lint pass is defined here because it touches parts of the `LateContext`
-// that we don't want to expose. It records the lint level at certain AST
-// nodes, so that the variant size difference check in trans can call
-// `raw_emit_lint`.
-
-pub struct GatherNodeLevels;
-
-impl LintPass for GatherNodeLevels {
-    fn get_lints(&self) -> LintArray {
-        lint_array!()
-    }
-}
-
-impl LateLintPass for GatherNodeLevels {
-    fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
-        match it.node {
-            hir::ItemEnum(..) => {
-                let lint_id = LintId::of(builtin::VARIANT_SIZE_DIFFERENCES);
-                let lvlsrc = cx.lints.get_level_source(lint_id);
-                match lvlsrc {
-                    (lvl, _) if lvl != Allow => {
-                        cx.node_levels.borrow_mut()
-                            .insert((it.id, lint_id), lvlsrc);
-                    },
-                    _ => { }
-                }
-            },
-            _ => { }
-        }
-    }
-}
-
 enum CheckLintNameResult {
     Ok,
     // Lint doesn't exist
@@ -1234,8 +1196,6 @@ pub fn check_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
         }
     }
 
-    *tcx.node_lint_levels.borrow_mut() = cx.node_levels.into_inner();
-
     // Put the lint store back in the session.
     mem::replace(&mut *tcx.sess.lint_store.borrow_mut(), cx.lints);
 }

src/librustc/lint/mod.rs

@@ -41,7 +41,7 @@ use hir;
 
 pub use lint::context::{LateContext, EarlyContext, LintContext, LintStore,
                         raw_emit_lint, check_crate, check_ast_crate, gather_attrs,
-                        raw_struct_lint, GatherNodeLevels, FutureIncompatibleInfo};
+                        raw_struct_lint, FutureIncompatibleInfo};
 
 /// Specification of a single lint.
 #[derive(Copy, Clone, Debug)]

src/librustc/session/config.rs

@@ -727,8 +727,6 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
         "load extra plugins"),
     unstable_options: bool = (false, parse_bool,
           "adds unstable command line options to rustc interface"),
-    print_enum_sizes: bool = (false, parse_bool,
-          "print the size of enums and their variants"),
     force_overflow_checks: Option<bool> = (None, parse_opt_bool,
           "force overflow checks on or off"),
     force_dropflag_checks: Option<bool> = (None, parse_opt_bool,

src/librustc/session/mod.rs

@@ -304,9 +304,6 @@ impl Session {
     pub fn unstable_options(&self) -> bool {
         self.opts.debugging_opts.unstable_options
     }
-    pub fn print_enum_sizes(&self) -> bool {
-        self.opts.debugging_opts.print_enum_sizes
-    }
     pub fn nonzeroing_move_hints(&self) -> bool {
         self.opts.debugging_opts.enable_nonzeroing_move_hints
     }

src/librustc/ty/context.rs

@@ -12,7 +12,6 @@
 
 use dep_graph::{DepGraph, DepTrackingMap};
 use session::Session;
-use lint;
 use middle;
 use middle::cstore::LOCAL_CRATE;
 use hir::def::DefMap;
@@ -415,9 +414,6 @@ pub struct GlobalCtxt<'tcx> {
     /// Cache used by const_eval when decoding extern const fns
     pub extern_const_fns: RefCell<DefIdMap<NodeId>>,
 
-    pub node_lint_levels: RefCell<FnvHashMap<(NodeId, lint::LintId),
-                                              lint::LevelSource>>,
-
     /// Maps any item's def-id to its stability index.
     pub stability: RefCell<stability::Index<'tcx>>,
 
@@ -726,7 +722,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             populated_external_primitive_impls: RefCell::new(DefIdSet()),
             extern_const_statics: RefCell::new(DefIdMap()),
             extern_const_fns: RefCell::new(DefIdMap()),
-            node_lint_levels: RefCell::new(FnvHashMap()),
             stability: RefCell::new(stability),
             selection_cache: traits::SelectionCache::new(),
             evaluation_cache: traits::EvaluationCache::new(),

src/librustc_lint/lib.rs

@@ -108,6 +108,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
                  HardwiredLints,
                  WhileTrue,
                  ImproperCTypes,
+                 VariantSizeDifferences,
                  BoxPointers,
                  UnusedAttributes,
                  PathStatements,
@@ -209,9 +210,6 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
         },
         ]);
 
-    // We have one lint pass defined specially
-    store.register_late_pass(sess, false, box lint::GatherNodeLevels);
-
     // Register renamed and removed lints
     store.register_renamed("unknown_features", "unused_features");
     store.register_removed("unsigned_negation", "replaced by negate_unsigned feature gate");

src/librustc_lint/types.rs

@@ -13,6 +13,8 @@
 use rustc::hir::def_id::DefId;
 use rustc::ty::subst::Substs;
 use rustc::ty::{self, Ty, TyCtxt};
+use rustc::ty::layout::{Layout, Primitive};
+use rustc::traits::ProjectionMode;
 use middle::const_val::ConstVal;
 use rustc_const_eval::eval_const_expr_partial;
 use rustc_const_eval::EvalHint::ExprTypeChecked;
@@ -76,6 +78,12 @@ declare_lint! {
     "shift exceeds the type's number of bits"
 }
 
+declare_lint! {
+    VARIANT_SIZE_DIFFERENCES,
+    Allow,
+    "detects enums with widely varying variant sizes"
+}
+
 #[derive(Copy, Clone)]
 pub struct TypeLimits {
     /// Id of the last visited negated expression
@@ -675,3 +683,64 @@ impl LateLintPass for ImproperCTypes {
         }
     }
 }
+
+pub struct VariantSizeDifferences;
+
+impl LintPass for VariantSizeDifferences {
+    fn get_lints(&self) -> LintArray {
+        lint_array!(VARIANT_SIZE_DIFFERENCES)
+    }
+}
+
+impl LateLintPass for VariantSizeDifferences {
+    fn check_item(&mut self, cx: &LateContext, it: &hir::Item) {
+        if let hir::ItemEnum(ref enum_definition, ref gens) = it.node {
+            if gens.ty_params.is_empty() {  // sizes only make sense for non-generic types
+                let t = cx.tcx.node_id_to_type(it.id);
+                let layout = cx.tcx.normalizing_infer_ctxt(ProjectionMode::Any).enter(|infcx| {
+                    t.layout(&infcx).unwrap_or_else(|e| {
+                        bug!("failed to get layout for `{}`: {}", t, e)
+                    })
+                });
+
+                if let Layout::General { ref variants, ref size, discr, .. } = *layout {
+                    let discr_size = Primitive::Int(discr).size(&cx.tcx.data_layout).bytes();
+
+                    debug!("enum `{}` is {} bytes large", t, size.bytes());
+
+                    let (largest, slargest, largest_index) = enum_definition.variants
+                        .iter()
+                        .zip(variants)
+                        .map(|(variant, variant_layout)| {
+                            // Subtract the size of the enum discriminant
+                            let bytes = variant_layout.min_size().bytes()
+                                                                 .saturating_sub(discr_size);
+
+                            debug!("- variant `{}` is {} bytes large", variant.node.name, bytes);
+                            bytes
+                        })
+                        .enumerate()
+                        .fold((0, 0, 0),
+                            |(l, s, li), (idx, size)|
+                                if size > l {
+                                    (size, l, idx)
+                                } else if size > s {
+                                    (l, size, li)
+                                } else {
+                                    (l, s, li)
+                                }
+                        );
+
+                    // we only warn if the largest variant is at least thrice as large as
+                    // the second-largest.
+                    if largest > slargest * 3 && slargest > 0 {
+                        cx.span_lint(VARIANT_SIZE_DIFFERENCES,
+                                     enum_definition.variants[largest_index].span,
+                                     &format!("enum variant is more than three times larger \
+                                               ({} bytes) than the next largest", largest));
+                    }
+                }
+            }
+        }
+    }
+}