OpenE2K/rust
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Move variant_size_differences out of trans

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

Closes #34018
This commit is contained in:
Jonas Schievink 2016-06-02 23:43:16 +02:00
parent 6871b3f240
commit f5d29a3b59
11 changed files with 123 additions and 241 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/librustc/lint/builtin.rs
View File

@ -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,

36
src/librustc/lint/context.rs
View File

@ -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;
@ -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

2
src/librustc/lint/mod.rs
View File

@ -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)]

2
src/librustc/session/config.rs
View File

@ -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,

3
src/librustc/session/mod.rs
View File

@ -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
}

4
src/librustc_lint/lib.rs
View File

@ -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");

68
src/librustc_lint/types.rs
View File

@ -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;
@ -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
@ -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));
}
}
}
}
}
}

184
src/librustc_trans/base.rs
View File

@ -30,7 +30,6 @@ use super::ModuleTranslation;
use back::link;
use back::linker::LinkerInfo;
use lint;
use llvm::{BasicBlockRef, Linkage, ValueRef, Vector, get_param};
use llvm;
use rustc::cfg;
@ -75,7 +74,7 @@ use expr;
use glue;
use inline;
use machine;
use machine::{llalign_of_min, llsize_of, llsize_of_real};
use machine::{llalign_of_min, llsize_of};
use meth;
use mir;
use monomorphize::{self, Instance};
@ -86,7 +85,6 @@ use trans_item::TransItem;
use tvec;
use type_::Type;
use type_of;
use type_of::*;
use value::Value;
use Disr;
use util::common::indenter;
@ -2074,87 +2072,6 @@ pub fn trans_ctor_shim<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
fcx.finish(bcx, DebugLoc::None);
}
fn enum_variant_size_lint(ccx: &CrateContext, enum_def: &hir::EnumDef, sp: Span, id: ast::NodeId) {
let mut sizes = Vec::new(); // does no allocation if no pushes, thankfully
let print_info = ccx.sess().print_enum_sizes();
let levels = ccx.tcx().node_lint_levels.borrow();
let lint_id = lint::LintId::of(lint::builtin::VARIANT_SIZE_DIFFERENCES);
let lvlsrc = levels.get(&(id, lint_id));
let is_allow = lvlsrc.map_or(true, |&(lvl, _)| lvl == lint::Allow);
if is_allow && !print_info {
// we're not interested in anything here
return;
}
let ty = ccx.tcx().node_id_to_type(id);
let avar = adt::represent_type(ccx, ty);
match *avar {
adt::General(_, ref variants, _) => {
for var in variants {
let mut size = 0;
for field in var.fields.iter().skip(1) {
// skip the discriminant
size += llsize_of_real(ccx, sizing_type_of(ccx, *field));
}
sizes.push(size);
}
},
_ => { /* its size is either constant or unimportant */ }
}
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)
}
);
// FIXME(#30505) Should use logging for this.
if print_info {
let llty = type_of::sizing_type_of(ccx, ty);
let sess = &ccx.tcx().sess;
sess.span_note_without_error(sp,
&format!("total size: {} bytes", llsize_of_real(ccx, llty)));
match *avar {
adt::General(..) => {
for (i, var) in enum_def.variants.iter().enumerate() {
ccx.tcx()
.sess
.span_note_without_error(var.span,
&format!("variant data: {} bytes", sizes[i]));
}
}
_ => {}
}
}
// we only warn if the largest variant is at least thrice as large as
// the second-largest.
if !is_allow && largest > slargest * 3 && slargest > 0 {
// Use lint::raw_emit_lint rather than sess.add_lint because the lint-printing
// pass for the latter already ran.
lint::raw_struct_lint(&ccx.tcx().sess,
&ccx.tcx().sess.lint_store.borrow(),
lint::builtin::VARIANT_SIZE_DIFFERENCES,
*lvlsrc.unwrap(),
Some(sp),
&format!("enum variant is more than three times larger ({} bytes) \
than the next largest (ignoring padding)",
largest))
.span_note(enum_def.variants[largest_index].span,
"this variant is the largest")
.emit();
}
}
pub fn llvm_linkage_by_name(name: &str) -> Option<Linkage> {
// Use the names from src/llvm/docs/LangRef.rst here. Most types are only
// applicable to variable declarations and may not really make sense for
@ -2194,26 +2111,6 @@ pub fn set_link_section(ccx: &CrateContext,
}
}
fn trans_item(ccx: &CrateContext, item: &hir::Item) {
let _icx = push_ctxt("trans_item");
match item.node {
hir::ItemEnum(ref enum_definition, ref gens) => {
if gens.ty_params.is_empty() {
// sizes only make sense for non-generic types
enum_variant_size_lint(ccx, enum_definition, item.span, item.id);
}
}
hir::ItemFn(..) |
hir::ItemImpl(..) |
hir::ItemStatic(..) => {
// Don't do anything here. Translation has been moved to
// being "collector-driven".
}
_ => {}
}
}
/// Create the `main` function which will initialise the rust runtime and call
/// users’ main function.
pub fn maybe_create_entry_wrapper(ccx: &CrateContext) {
@ -2659,19 +2556,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
symbol_names_test::report_symbol_names(&shared_ccx);
{
let ccx = crate_context_list.get_ccx(0);
// FIXME: #34018
// At this point, we only walk the HIR for running
// enum_variant_size_lint(). This should arguably be moved somewhere
// else.
{
intravisit::walk_mod(&mut TransItemsWithinModVisitor { ccx: &ccx }, &krate.module);
krate.visit_all_items(&mut TransModVisitor { ccx: &ccx });
}
}
if shared_ccx.sess().trans_stats() {
let stats = shared_ccx.stats();
println!("--- trans stats ---");
@ -2758,72 +2642,6 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}
}
/// We visit all the items in the krate and translate them. We do
/// this in two walks. The first walk just finds module items. It then
/// walks the full contents of those module items and translates all
/// the items within. Note that this entire process is O(n). The
/// reason for this two phased walk is that each module is
/// (potentially) placed into a distinct codegen-unit. This walk also
/// ensures that the immediate contents of each module is processed
/// entirely before we proceed to find more modules, helping to ensure
/// an equitable distribution amongst codegen-units.
pub struct TransModVisitor<'a, 'tcx: 'a> {
pub ccx: &'a CrateContext<'a, 'tcx>,
}
impl<'a, 'tcx, 'v> Visitor<'v> for TransModVisitor<'a, 'tcx> {
fn visit_item(&mut self, i: &hir::Item) {
match i.node {
hir::ItemMod(_) => {
let item_ccx = self.ccx.rotate();
intravisit::walk_item(&mut TransItemsWithinModVisitor { ccx: &item_ccx }, i);
}
_ => { }
}
}
}
/// Translates all the items within a given module. Expects owner to
/// invoke `walk_item` on a module item. Ignores nested modules.
pub struct TransItemsWithinModVisitor<'a, 'tcx: 'a> {
pub ccx: &'a CrateContext<'a, 'tcx>,
}
impl<'a, 'tcx, 'v> Visitor<'v> for TransItemsWithinModVisitor<'a, 'tcx> {
fn visit_nested_item(&mut self, item_id: hir::ItemId) {
self.visit_item(self.ccx.tcx().map.expect_item(item_id.id));
}
fn visit_item(&mut self, i: &hir::Item) {
match i.node {
hir::ItemMod(..) => {
// skip modules, they will be uncovered by the TransModVisitor
}
_ => {
let def_id = self.ccx.tcx().map.local_def_id(i.id);
let tcx = self.ccx.tcx();
// Create a subtask for trans'ing a particular item. We are
// giving `trans_item` access to this item, so also record a read.
tcx.dep_graph.with_task(DepNode::TransCrateItem(def_id), || {
tcx.dep_graph.read(DepNode::Hir(def_id));
// We are going to be accessing various tables
// generated by TypeckItemBody; we also assume
// that the body passes type check. These tables
// are not individually tracked, so just register
// a read here.
tcx.dep_graph.read(DepNode::TypeckItemBody(def_id));
trans_item(self.ccx, i);
});
intravisit::walk_item(self, i);
}
}
}
}
fn collect_and_partition_translation_items<'a, 'tcx>(scx: &SharedCrateContext<'a, 'tcx>)
-> (Vec<CodegenUnit<'tcx>>, SymbolMap<'tcx>) {
let time_passes = scx.sess().time_passes();

8
src/librustc_trans/context.rs
View File

@ -219,14 +219,6 @@ impl<'a, 'tcx: 'a> CrateContextList<'a, 'tcx> {
}
}
pub fn get_ccx<'b>(&'b self, index: usize) -> CrateContext<'b, 'tcx> {
CrateContext {
shared: self.shared,
index: index,
local_ccxs: &self.local_ccxs[..],
}
}
pub fn shared(&self) -> &'a SharedCrateContext<'a, 'tcx> {
self.shared
}

32
src/librustc_trans/trans_item.rs
View File

@ -28,6 +28,7 @@ use rustc::hir::map as hir_map;
use rustc::hir::def_id::DefId;
use rustc::ty::{self, Ty, TyCtxt, TypeFoldable};
use rustc::ty::subst;
use rustc::dep_graph::DepNode;
use std::hash::{Hash, Hasher};
use syntax::ast::{self, NodeId};
use syntax::{attr,errors};
@ -73,6 +74,8 @@ impl<'a, 'tcx> TransItem<'tcx> {
self.to_raw_string(),
ccx.codegen_unit().name);
self.register_reads(ccx);
match *self {
TransItem::Static(node_id) => {
let item = ccx.tcx().map.expect_item(node_id);
@ -99,6 +102,35 @@ impl<'a, 'tcx> TransItem<'tcx> {
ccx.codegen_unit().name);
}
/// If necessary, creates a subtask for trans'ing a particular item and registers reads on
/// `TypeckItemBody` and `Hir`.
fn register_reads(&self, ccx: &CrateContext<'a, 'tcx>) {
let tcx = ccx.tcx();
let def_id = match *self {
TransItem::Static(node_id) => {
tcx.map.local_def_id(node_id)
}
TransItem::Fn(instance) => {
instance.def
}
TransItem::DropGlue(_) => {
// Nothing to track for drop glue
return;
}
};
tcx.dep_graph.with_task(DepNode::TransCrateItem(def_id), || {
tcx.dep_graph.read(DepNode::Hir(def_id));
// We are going to be accessing various tables
// generated by TypeckItemBody; we also assume
// that the body passes type check. These tables
// are not individually tracked, so just register
// a read here.
tcx.dep_graph.read(DepNode::TypeckItemBody(def_id));
});
}
pub fn predefine(&self,
ccx: &CrateContext<'a, 'tcx>,
linkage: llvm::Linkage) {

18
src/test/compile-fail/variant-size-differences.rs Normal file
View File

@ -0,0 +1,18 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![deny(variant_size_differences)]
enum _En {
V0(u8),
VBig([u8; 1024]), //~ ERROR variant is more than three times larger
}
fn main() {}