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

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
This commit is contained in:
bors 2016-07-12 01:06:34 -07:00 committed by GitHub
parent 2539c15b49 fd2b65ee70
commit 5c69a4f619
12 changed files with 135 additions and 254 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,

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

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

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
}

5
src/librustc/ty/context.rs
View File

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

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

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

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
}

43
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,46 @@ 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) => {
if let Some(node) = tcx.map.as_local_node_id(instance.def) {
if let hir_map::Node::NodeItem(_) = tcx.map.get(node) {
// This already is a "real" item
instance.def
} else {
// Get the enclosing item and register a read on it
tcx.map.get_parent_did(node)
}
} else {
// Translating an inlined item from another crate? Don't track anything.
return;
}
}
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() {}