Skip to content

Commit

Permalink
Move variant_size_differences out of trans
Browse files Browse the repository at this point in the history
Also enhances the error message a bit, fixes #30505 on the way, and adds
a test (which was missing).

Closes #34018
  • Loading branch information
jonas-schievink committed Jul 10, 2016
1 parent 6871b3f commit f5d29a3
Show file tree
Hide file tree
Showing 11 changed files with 123 additions and 241 deletions.
7 changes: 0 additions & 7 deletions src/librustc/lint/builtin.rs
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
36 changes: 2 additions & 34 deletions src/librustc/lint/context.rs
Expand Up @@ -29,8 +29,8 @@ 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;
Expand Down Expand Up @@ -1064,38 +1064,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
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/lint/mod.rs
Expand Up @@ -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)]
Expand Down
2 changes: 0 additions & 2 deletions src/librustc/session/config.rs
Expand Up @@ -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,
Expand Down
3 changes: 0 additions & 3 deletions src/librustc/session/mod.rs
Expand Up @@ -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
}
Expand Down
4 changes: 1 addition & 3 deletions src/librustc_lint/lib.rs
Expand Up @@ -108,6 +108,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
HardwiredLints,
WhileTrue,
ImproperCTypes,
VariantSizeDifferences,
BoxPointers,
UnusedAttributes,
PathStatements,
Expand Down Expand Up @@ -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");
Expand Down
68 changes: 68 additions & 0 deletions src/librustc_lint/types.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -77,6 +79,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
Expand Down Expand Up @@ -676,3 +684,63 @@ 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 mut sizes = vec![];
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());

for (variant, variant_layout) in enum_definition.variants.iter().zip(variants) {
// Subtract the size of the enum discriminant
let bytes = variant_layout.min_size().bytes().saturating_sub(discr_size);
sizes.push(bytes);

debug!("- variant `{}` is {} bytes large", variant.node.name, bytes);
}

let (largest, slargest, largest_index) = sizes.iter()
.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));
}
}
}
}
}
}

0 comments on commit f5d29a3

Please sign in to comment.