Skip to content

Commit

Permalink
Auto merge of #63489 - Centril:rollup-uuf6z1s, r=Centril
Browse files Browse the repository at this point in the history
Rollup of 9 pull requests

Successful merges:

 - #62108 (Use sharded maps for queries)
 - #63297 (Improve pointer offset method docs)
 - #63406 (Suggest using a qualified path in patterns with inconsistent bindings)
 - #63431 (Revert "Simplify MIR generation for logical ops")
 - #63449 (resolve: Remove remaining special cases from built-in macros)
 - #63461 (docs: add stdlib env::var(_os) panic)
 - #63473 (Regression test for #56870)
 - #63474 (Add tests for issue #53598 and #57700)
 - #63480 (Fixes #63477)

Failed merges:

r? @ghost
  • Loading branch information
bors committed Aug 12, 2019
2 parents 72f8043 + 9d29719 commit c01be67
Show file tree
Hide file tree
Showing 28 changed files with 499 additions and 179 deletions.
162 changes: 128 additions & 34 deletions src/libcore/ptr/mod.rs

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions src/librustc/ty/query/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::util::profiling::ProfileCategory;
use std::borrow::Cow;
use std::hash::Hash;
use std::fmt::Debug;
use rustc_data_structures::sync::Lock;
use rustc_data_structures::sharded::Sharded;
use rustc_data_structures::fingerprint::Fingerprint;
use crate::ich::StableHashingContext;

Expand All @@ -34,7 +34,7 @@ pub(crate) trait QueryAccessors<'tcx>: QueryConfig<'tcx> {
fn query(key: Self::Key) -> Query<'tcx>;

// Don't use this method to access query results, instead use the methods on TyCtxt
fn query_cache<'a>(tcx: TyCtxt<'tcx>) -> &'a Lock<QueryCache<'tcx, Self>>;
fn query_cache<'a>(tcx: TyCtxt<'tcx>) -> &'a Sharded<QueryCache<'tcx, Self>>;

fn to_dep_node(tcx: TyCtxt<'tcx>, key: &Self::Key) -> DepNode;

Expand Down
6 changes: 3 additions & 3 deletions src/librustc/ty/query/on_disk_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1062,9 +1062,9 @@ where
::std::any::type_name::<Q>());

time_ext(tcx.sess.time_extended(), Some(tcx.sess), desc, || {
let map = Q::query_cache(tcx).borrow();
assert!(map.active.is_empty());
for (key, entry) in map.results.iter() {
let shards = Q::query_cache(tcx).lock_shards();
assert!(shards.iter().all(|shard| shard.active.is_empty()));
for (key, entry) in shards.iter().flat_map(|shard| shard.results.iter()) {
if Q::cache_on_disk(tcx, key.clone(), Some(&entry.value)) {
let dep_node = SerializedDepNodeIndex::new(entry.index.index());

Expand Down
44 changes: 23 additions & 21 deletions src/librustc/ty/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use errors::Diagnostic;
use errors::FatalError;
use rustc_data_structures::fx::{FxHashMap};
use rustc_data_structures::sync::{Lrc, Lock};
use rustc_data_structures::sharded::Sharded;
use rustc_data_structures::thin_vec::ThinVec;
#[cfg(not(parallel_compiler))]
use rustc_data_structures::cold_path;
Expand Down Expand Up @@ -90,7 +91,7 @@ macro_rules! profq_query_msg {
/// A type representing the responsibility to execute the job in the `job` field.
/// This will poison the relevant query if dropped.
pub(super) struct JobOwner<'a, 'tcx, Q: QueryDescription<'tcx>> {
cache: &'a Lock<QueryCache<'tcx, Q>>,
cache: &'a Sharded<QueryCache<'tcx, Q>>,
key: Q::Key,
job: Lrc<QueryJob<'tcx>>,
}
Expand All @@ -107,7 +108,7 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> {
pub(super) fn try_get(tcx: TyCtxt<'tcx>, span: Span, key: &Q::Key) -> TryGetJob<'a, 'tcx, Q> {
let cache = Q::query_cache(tcx);
loop {
let mut lock = cache.borrow_mut();
let mut lock = cache.get_shard_by_value(key).lock();
if let Some(value) = lock.results.get(key) {
profq_msg!(tcx, ProfileQueriesMsg::CacheHit);
tcx.sess.profiler(|p| p.record_query_hit(Q::NAME));
Expand Down Expand Up @@ -191,7 +192,7 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> JobOwner<'a, 'tcx, Q> {

let value = QueryValue::new(result.clone(), dep_node_index);
{
let mut lock = cache.borrow_mut();
let mut lock = cache.get_shard_by_value(&key).lock();
lock.active.remove(&key);
lock.results.insert(key, value);
}
Expand All @@ -215,7 +216,8 @@ impl<'a, 'tcx, Q: QueryDescription<'tcx>> Drop for JobOwner<'a, 'tcx, Q> {
#[cold]
fn drop(&mut self) {
// Poison the query so jobs waiting on it panic
self.cache.borrow_mut().active.insert(self.key.clone(), QueryResult::Poisoned);
let shard = self.cache.get_shard_by_value(&self.key);
shard.lock().active.insert(self.key.clone(), QueryResult::Poisoned);
// Also signal the completion of the job, so waiters
// will continue execution
self.job.signal_complete();
Expand Down Expand Up @@ -708,7 +710,7 @@ macro_rules! define_queries_inner {
use std::mem;
#[cfg(parallel_compiler)]
use ty::query::job::QueryResult;
use rustc_data_structures::sync::Lock;
use rustc_data_structures::sharded::Sharded;
use crate::{
rustc_data_structures::stable_hasher::HashStable,
rustc_data_structures::stable_hasher::StableHasherResult,
Expand Down Expand Up @@ -740,18 +742,17 @@ macro_rules! define_queries_inner {
pub fn collect_active_jobs(&self) -> Vec<Lrc<QueryJob<$tcx>>> {
let mut jobs = Vec::new();

// We use try_lock here since we are only called from the
// We use try_lock_shards here since we are only called from the
// deadlock handler, and this shouldn't be locked.
$(
jobs.extend(
self.$name.try_lock().unwrap().active.values().filter_map(|v|
if let QueryResult::Started(ref job) = *v {
Some(job.clone())
} else {
None
}
)
);
let shards = self.$name.try_lock_shards().unwrap();
jobs.extend(shards.iter().flat_map(|shard| shard.active.values().filter_map(|v|
if let QueryResult::Started(ref job) = *v {
Some(job.clone())
} else {
None
}
)));
)*

jobs
Expand All @@ -773,26 +774,27 @@ macro_rules! define_queries_inner {

fn stats<'tcx, Q: QueryConfig<'tcx>>(
name: &'static str,
map: &QueryCache<'tcx, Q>
map: &Sharded<QueryCache<'tcx, Q>>,
) -> QueryStats {
let map = map.lock_shards();
QueryStats {
name,
#[cfg(debug_assertions)]
cache_hits: map.cache_hits,
cache_hits: map.iter().map(|shard| shard.cache_hits).sum(),
#[cfg(not(debug_assertions))]
cache_hits: 0,
key_size: mem::size_of::<Q::Key>(),
key_type: type_name::<Q::Key>(),
value_size: mem::size_of::<Q::Value>(),
value_type: type_name::<Q::Value>(),
entry_count: map.results.len(),
entry_count: map.iter().map(|shard| shard.results.len()).sum(),
}
}

$(
queries.push(stats::<queries::$name<'_>>(
stringify!($name),
&*self.$name.lock()
&self.$name,
));
)*

Expand Down Expand Up @@ -967,7 +969,7 @@ macro_rules! define_queries_inner {
}

#[inline(always)]
fn query_cache<'a>(tcx: TyCtxt<$tcx>) -> &'a Lock<QueryCache<$tcx, Self>> {
fn query_cache<'a>(tcx: TyCtxt<$tcx>) -> &'a Sharded<QueryCache<$tcx, Self>> {
&tcx.queries.$name
}

Expand Down Expand Up @@ -1099,7 +1101,7 @@ macro_rules! define_queries_struct {
providers: IndexVec<CrateNum, Providers<$tcx>>,
fallback_extern_providers: Box<Providers<$tcx>>,

$($(#[$attr])* $name: Lock<QueryCache<$tcx, queries::$name<$tcx>>>,)*
$($(#[$attr])* $name: Sharded<QueryCache<$tcx, queries::$name<$tcx>>>,)*
}
};
}
Expand Down
53 changes: 30 additions & 23 deletions src/librustc_mir/build/expr/into.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,59 +79,66 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
ExprKind::LogicalOp { op, lhs, rhs } => {
// And:
//
// [block: If(lhs)] -true-> [else_block: dest = (rhs)]
// | (false)
// [shortcurcuit_block: dest = false]
// [block: If(lhs)] -true-> [else_block: If(rhs)] -true-> [true_block]
// | | (false)
// +----------false-----------+------------------> [false_block]
//
// Or:
//
// [block: If(lhs)] -false-> [else_block: dest = (rhs)]
// | (true)
// [shortcurcuit_block: dest = true]
// [block: If(lhs)] -false-> [else_block: If(rhs)] -true-> [true_block]
// | (true) | (false)
// [true_block] [false_block]

let (shortcircuit_block, mut else_block, join_block) = (
let (true_block, false_block, mut else_block, join_block) = (
this.cfg.start_new_block(),
this.cfg.start_new_block(),
this.cfg.start_new_block(),
this.cfg.start_new_block(),
);

let lhs = unpack!(block = this.as_local_operand(block, lhs));
let blocks = match op {
LogicalOp::And => (else_block, shortcircuit_block),
LogicalOp::Or => (shortcircuit_block, else_block),
LogicalOp::And => (else_block, false_block),
LogicalOp::Or => (true_block, else_block),
};
let term = TerminatorKind::if_(this.hir.tcx(), lhs, blocks.0, blocks.1);
this.cfg.terminate(block, source_info, term);

let rhs = unpack!(else_block = this.as_local_operand(else_block, rhs));
let term = TerminatorKind::if_(this.hir.tcx(), rhs, true_block, false_block);
this.cfg.terminate(else_block, source_info, term);

this.cfg.push_assign_constant(
shortcircuit_block,
true_block,
source_info,
destination,
Constant {
span: expr_span,
ty: this.hir.bool_ty(),
user_ty: None,
literal: match op {
LogicalOp::And => this.hir.false_literal(),
LogicalOp::Or => this.hir.true_literal(),
},
literal: this.hir.true_literal(),
},
);
this.cfg.terminate(
shortcircuit_block,

this.cfg.push_assign_constant(
false_block,
source_info,
TerminatorKind::Goto { target: join_block },
destination,
Constant {
span: expr_span,
ty: this.hir.bool_ty(),
user_ty: None,
literal: this.hir.false_literal(),
},
);

let rhs = unpack!(else_block = this.as_local_operand(else_block, rhs));
this.cfg.push_assign(
else_block,
this.cfg.terminate(
true_block,
source_info,
destination,
Rvalue::Use(rhs),
TerminatorKind::Goto { target: join_block },
);
this.cfg.terminate(
else_block,
false_block,
source_info,
TerminatorKind::Goto { target: join_block },
);
Expand Down
7 changes: 0 additions & 7 deletions src/librustc_resolve/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,6 @@ impl<'a> Resolver<'a> {
};
if let Some(id) = self.definitions.as_local_node_id(def_id) {
self.local_macro_def_scopes[&id]
} else if self.is_builtin_macro(Some(def_id)) {
self.injected_crate.unwrap_or(self.graph_root)
} else {
let module_def_id = ty::DefIdTree::parent(&*self, def_id).unwrap();
self.get_module(module_def_id)
Expand Down Expand Up @@ -596,11 +594,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
};

self.r.populate_module_if_necessary(module);
if let Some(name) = self.r.session.parse_sess.injected_crate_name.try_get() {
if name.as_str() == ident.name.as_str() {
self.r.injected_crate = Some(module);
}
}

let used = self.process_legacy_macro_imports(item, module);
let binding =
Expand Down
21 changes: 16 additions & 5 deletions src/librustc_resolve/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use syntax_pos::{BytePos, Span, MultiSpan};

use crate::resolve_imports::{ImportDirective, ImportDirectiveSubclass, ImportResolver};
use crate::{path_names_to_string, KNOWN_TOOLS};
use crate::{CrateLint, LegacyScope, Module, ModuleOrUniformRoot};
use crate::{BindingError, CrateLint, LegacyScope, Module, ModuleOrUniformRoot};
use crate::{PathResult, ParentScope, ResolutionError, Resolver, Scope, ScopeSet, Segment};

type Res = def::Res<ast::NodeId>;
Expand Down Expand Up @@ -207,21 +207,32 @@ impl<'a> Resolver<'a> {
err
}
ResolutionError::VariableNotBoundInPattern(binding_error) => {
let target_sp = binding_error.target.iter().cloned().collect::<Vec<_>>();
let BindingError { name, target, origin, could_be_path } = binding_error;

let target_sp = target.iter().copied().collect::<Vec<_>>();
let origin_sp = origin.iter().copied().collect::<Vec<_>>();

let msp = MultiSpan::from_spans(target_sp.clone());
let msg = format!("variable `{}` is not bound in all patterns", binding_error.name);
let msg = format!("variable `{}` is not bound in all patterns", name);
let mut err = self.session.struct_span_err_with_code(
msp,
&msg,
DiagnosticId::Error("E0408".into()),
);
for sp in target_sp {
err.span_label(sp, format!("pattern doesn't bind `{}`", binding_error.name));
err.span_label(sp, format!("pattern doesn't bind `{}`", name));
}
let origin_sp = binding_error.origin.iter().cloned();
for sp in origin_sp {
err.span_label(sp, "variable not in all patterns");
}
if *could_be_path {
let help_msg = format!(
"if you meant to match on a variant or a `const` item, consider \
making the path in the pattern qualified: `?::{}`",
name,
);
err.span_help(span, &help_msg);
}
err
}
ResolutionError::VariableBoundWithDifferentMode(variable_name,
Expand Down
Loading

0 comments on commit c01be67

Please sign in to comment.