Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use a Set<T> instead of a Map<T, bool> #45736

Merged
merged 1 commit into from
Nov 9, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/librustc/middle/expr_use_visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ use std::rc::Rc;
use syntax::ast;
use syntax::ptr::P;
use syntax_pos::Span;
use util::nodemap::ItemLocalMap;
use util::nodemap::ItemLocalSet;

///////////////////////////////////////////////////////////////////////////
// The Delegate trait
Expand Down Expand Up @@ -279,7 +279,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx, 'tcx> {
param_env: ty::ParamEnv<'tcx>,
region_scope_tree: &'a region::ScopeTree,
tables: &'a ty::TypeckTables<'tcx>,
rvalue_promotable_map: Option<Rc<ItemLocalMap<bool>>>)
rvalue_promotable_map: Option<Rc<ItemLocalSet>>)
-> Self
{
ExprUseVisitor {
Expand Down
8 changes: 4 additions & 4 deletions src/librustc/middle/mem_categorization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ use syntax_pos::Span;

use std::fmt;
use std::rc::Rc;
use util::nodemap::ItemLocalMap;
use util::nodemap::ItemLocalSet;

#[derive(Clone, PartialEq)]
pub enum Categorization<'tcx> {
Expand Down Expand Up @@ -286,7 +286,7 @@ pub struct MemCategorizationContext<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
pub tcx: TyCtxt<'a, 'gcx, 'tcx>,
pub region_scope_tree: &'a region::ScopeTree,
pub tables: &'a ty::TypeckTables<'tcx>,
rvalue_promotable_map: Option<Rc<ItemLocalMap<bool>>>,
rvalue_promotable_map: Option<Rc<ItemLocalSet>>,
infcx: Option<&'a InferCtxt<'a, 'gcx, 'tcx>>,
}

Expand Down Expand Up @@ -395,7 +395,7 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx, 'tcx> {
pub fn new(tcx: TyCtxt<'a, 'tcx, 'tcx>,
region_scope_tree: &'a region::ScopeTree,
tables: &'a ty::TypeckTables<'tcx>,
rvalue_promotable_map: Option<Rc<ItemLocalMap<bool>>>)
rvalue_promotable_map: Option<Rc<ItemLocalSet>>)
-> MemCategorizationContext<'a, 'tcx, 'tcx> {
MemCategorizationContext {
tcx,
Expand Down Expand Up @@ -897,7 +897,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> {
expr_ty: Ty<'tcx>)
-> cmt<'tcx> {
let hir_id = self.tcx.hir.node_to_hir_id(id);
let promotable = self.rvalue_promotable_map.as_ref().map(|m| m[&hir_id.local_id])
let promotable = self.rvalue_promotable_map.as_ref().map(|m| m.contains(&hir_id.local_id))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only change in semantics. We do not panic anymore if local_id was never checked for rvalue promotion, we just return false

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, seems..ok

.unwrap_or(false);

// Always promote `[T; 0]` (even when e.g. borrowed mutably).
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/maps/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use ty::{self, CrateInherentImpls, Ty, TyCtxt};
use ty::layout::{Layout, LayoutError};
use ty::steal::Steal;
use ty::subst::Substs;
use util::nodemap::{DefIdSet, DefIdMap, ItemLocalMap};
use util::nodemap::{DefIdSet, DefIdMap, ItemLocalSet};
use util::common::{profq_msg, ProfileQueriesMsg};

use rustc_data_structures::indexed_set::IdxSetBuf;
Expand Down Expand Up @@ -236,7 +236,7 @@ define_maps! { <'tcx>
[] fn is_exported_symbol: IsExportedSymbol(DefId) -> bool,
[] fn item_body_nested_bodies: ItemBodyNestedBodies(DefId) -> ExternBodyNestedBodies,
[] fn const_is_rvalue_promotable_to_static: ConstIsRvaluePromotableToStatic(DefId) -> bool,
[] fn rvalue_promotable_map: RvaluePromotableMap(DefId) -> Rc<ItemLocalMap<bool>>,
[] fn rvalue_promotable_map: RvaluePromotableMap(DefId) -> Rc<ItemLocalSet>,
[] fn is_mir_available: IsMirAvailable(DefId) -> bool,
[] fn vtable_methods: vtable_methods_node(ty::PolyTraitRef<'tcx>)
-> Rc<Vec<Option<(DefId, &'tcx Substs<'tcx>)>>>,
Expand Down
2 changes: 2 additions & 0 deletions src/librustc/util/nodemap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,12 @@ pub type ItemLocalMap<T> = FxHashMap<ItemLocalId, T>;

pub type NodeSet = FxHashSet<ast::NodeId>;
pub type DefIdSet = FxHashSet<DefId>;
pub type ItemLocalSet = FxHashSet<ItemLocalId>;

pub fn NodeMap<T>() -> NodeMap<T> { FxHashMap() }
pub fn DefIdMap<T>() -> DefIdMap<T> { FxHashMap() }
pub fn ItemLocalMap<T>() -> ItemLocalMap<T> { FxHashMap() }
pub fn NodeSet() -> NodeSet { FxHashSet() }
pub fn DefIdSet() -> DefIdSet { FxHashSet() }
pub fn ItemLocalSet() -> ItemLocalSet { FxHashSet() }

16 changes: 9 additions & 7 deletions src/librustc_passes/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ use rustc::ty::maps::{queries, Providers};
use rustc::ty::subst::Substs;
use rustc::traits::Reveal;
use rustc::util::common::ErrorReported;
use rustc::util::nodemap::{ItemLocalMap, NodeSet};
use rustc::util::nodemap::{ItemLocalSet, NodeSet};
use rustc::lint::builtin::CONST_ERR;
use rustc::hir::{self, PatKind, RangeEnd};
use std::rc::Rc;
Expand Down Expand Up @@ -79,12 +79,12 @@ fn const_is_rvalue_promotable_to_static<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
.expect("rvalue_promotable_map invoked with non-local def-id");
let body_id = tcx.hir.body_owned_by(node_id);
let body_hir_id = tcx.hir.node_to_hir_id(body_id.node_id);
tcx.rvalue_promotable_map(def_id).contains_key(&body_hir_id.local_id)
tcx.rvalue_promotable_map(def_id).contains(&body_hir_id.local_id)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this exact thing in clippy, and it was very wrong. I'm fairly certain it was wrong here, too.

This checked whether the local_id was ever checked for rvalue promotion. Not whether it is rvalue promotable.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}

fn rvalue_promotable_map<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
def_id: DefId)
-> Rc<ItemLocalMap<bool>>
-> Rc<ItemLocalSet>
{
let outer_def_id = tcx.closure_base_def_id(def_id);
if outer_def_id != def_id {
Expand All @@ -100,7 +100,7 @@ fn rvalue_promotable_map<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
mut_rvalue_borrows: NodeSet(),
param_env: ty::ParamEnv::empty(Reveal::UserFacing),
identity_substs: Substs::empty(),
result_map: ItemLocalMap(),
result: ItemLocalSet(),
};

// `def_id` should be a `Body` owner
Expand All @@ -109,7 +109,7 @@ fn rvalue_promotable_map<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
let body_id = tcx.hir.body_owned_by(node_id);
visitor.visit_nested_body(body_id);

Rc::new(visitor.result_map)
Rc::new(visitor.result)
}

struct CheckCrateVisitor<'a, 'tcx: 'a> {
Expand All @@ -121,7 +121,7 @@ struct CheckCrateVisitor<'a, 'tcx: 'a> {
param_env: ty::ParamEnv<'tcx>,
identity_substs: &'tcx Substs<'tcx>,
tables: &'a ty::TypeckTables<'tcx>,
result_map: ItemLocalMap<bool>,
result: ItemLocalSet,
}

impl<'a, 'gcx> CheckCrateVisitor<'a, 'gcx> {
Expand Down Expand Up @@ -322,7 +322,9 @@ impl<'a, 'tcx> Visitor<'tcx> for CheckCrateVisitor<'a, 'tcx> {
}
}

self.result_map.insert(ex.hir_id.local_id, self.promotable);
if self.promotable {
self.result.insert(ex.hir_id.local_id);
}
self.promotable &= outer;
}
}
Expand Down