Skip to content

Commit

Permalink
Auto merge of rust-lang#89417 - Manishearth:rollup-j2gdu95, r=Manishe…
Browse files Browse the repository at this point in the history
…arth

Rollup of 7 pull requests

Successful merges:

 - rust-lang#88838 (Do not suggest importing inaccessible items)
 - rust-lang#89251 (Detect when negative literal indices are used and suggest appropriate code)
 - rust-lang#89321 (Rebase resume argument projections during state transform)
 - rust-lang#89327 (Pick one possible lifetime in case there are multiple choices)
 - rust-lang#89344 (Cleanup lower_generics_mut and make span be the bound itself)
 - rust-lang#89397 (Update `llvm` submodule to fix function name mangling on x86 Windows)
 - rust-lang#89412 (Add regression test for issues rust-lang#88969 and rust-lang#89119 )

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
  • Loading branch information
bors committed Oct 1, 2021
2 parents 4e4942d + 4571fea commit 69eb996
Show file tree
Hide file tree
Showing 28 changed files with 560 additions and 160 deletions.
63 changes: 38 additions & 25 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1328,32 +1328,45 @@ impl<'hir> LoweringContext<'_, 'hir> {
// keep track of the Span info. Now, `add_implicitly_sized` in `AstConv` checks both param bounds and
// where clauses for `?Sized`.
for pred in &generics.where_clause.predicates {
if let WherePredicate::BoundPredicate(ref bound_pred) = *pred {
'next_bound: for bound in &bound_pred.bounds {
if let GenericBound::Trait(_, TraitBoundModifier::Maybe) = *bound {
// Check if the where clause type is a plain type parameter.
match self
.resolver
.get_partial_res(bound_pred.bounded_ty.id)
.map(|d| (d.base_res(), d.unresolved_segments()))
{
Some((Res::Def(DefKind::TyParam, def_id), 0))
if bound_pred.bound_generic_params.is_empty() =>
{
for param in &generics.params {
if def_id == self.resolver.local_def_id(param.id).to_def_id() {
continue 'next_bound;
}
}
}
_ => {}
}
self.diagnostic().span_err(
bound_pred.bounded_ty.span,
"`?Trait` bounds are only permitted at the \
point where a type parameter is declared",
);
let bound_pred = match *pred {
WherePredicate::BoundPredicate(ref bound_pred) => bound_pred,
_ => continue,
};
let compute_is_param = || {
// Check if the where clause type is a plain type parameter.
match self
.resolver
.get_partial_res(bound_pred.bounded_ty.id)
.map(|d| (d.base_res(), d.unresolved_segments()))
{
Some((Res::Def(DefKind::TyParam, def_id), 0))
if bound_pred.bound_generic_params.is_empty() =>
{
generics
.params
.iter()
.find(|p| def_id == self.resolver.local_def_id(p.id).to_def_id())
.is_some()
}
// Either the `bounded_ty` is not a plain type parameter, or
// it's not found in the generic type parameters list.
_ => false,
}
};
// We only need to compute this once per `WherePredicate`, but don't
// need to compute this at all unless there is a Maybe bound.
let mut is_param: Option<bool> = None;
for bound in &bound_pred.bounds {
if !matches!(*bound, GenericBound::Trait(_, TraitBoundModifier::Maybe)) {
continue;
}
let is_param = *is_param.get_or_insert_with(compute_is_param);
if !is_param {
self.diagnostic().span_err(
bound.span(),
"`?Trait` bounds are only permitted at the \
point where a type parameter is declared",
);
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ pub trait ResolverAstLowering {
fn legacy_const_generic_args(&mut self, expr: &Expr) -> Option<Vec<usize>>;

/// Obtains resolution for a `NodeId` with a single resolution.
fn get_partial_res(&mut self, id: NodeId) -> Option<PartialRes>;
fn get_partial_res(&self, id: NodeId) -> Option<PartialRes>;

/// Obtains per-namespace resolutions for `use` statement with the given `NodeId`.
fn get_import_res(&mut self, id: NodeId) -> PerNS<Option<Res<NodeId>>>;
Expand Down
17 changes: 16 additions & 1 deletion compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,22 @@ impl<'tcx> RegionInferenceContext<'tcx> {
ty::ReVar(vid) => {
// Find something that we can name
let upper_bound = self.approx_universal_upper_bound(vid);
self.definitions[upper_bound].external_name.unwrap_or(region)
let upper_bound = &self.definitions[upper_bound];
match upper_bound.external_name {
Some(reg) => reg,
None => {
// Nothing exact found, so we pick the first one that we find.
let scc = self.constraint_sccs.scc(vid);
for vid in self.rev_scc_graph.as_ref().unwrap().upper_bounds(scc) {
match self.definitions[vid].external_name {
None => {}
Some(&ty::ReStatic) => {}
Some(region) => return region,
}
}
region
}
}
}
_ => region,
})
Expand Down
5 changes: 3 additions & 2 deletions compiler/rustc_mir_transform/src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,15 +342,16 @@ impl MutVisitor<'tcx> for TransformVisitor<'tcx> {
let source_info = data.terminator().source_info;
// We must assign the value first in case it gets declared dead below
data.statements.extend(self.make_state(state_idx, v, source_info));
let state = if let Some((resume, resume_arg)) = resume {
let state = if let Some((resume, mut resume_arg)) = resume {
// Yield
let state = 3 + self.suspension_points.len();

// The resume arg target location might itself be remapped if its base local is
// live across a yield.
let resume_arg =
if let Some(&(ty, variant, idx)) = self.remap.get(&resume_arg.local) {
self.make_field(variant, idx, ty)
replace_base(&mut resume_arg, self.make_field(variant, idx, ty), self.tcx);
resume_arg
} else {
resume_arg
};
Expand Down
140 changes: 109 additions & 31 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -973,7 +973,15 @@ impl<'a> Resolver<'a> {

let import_suggestions =
self.lookup_import_candidates(ident, Namespace::MacroNS, parent_scope, is_expected);
show_candidates(err, None, &import_suggestions, false, true);
show_candidates(
&self.definitions,
self.session,
err,
None,
&import_suggestions,
false,
true,
);

if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) {
let msg = format!("unsafe traits like `{}` should be implemented explicitly", ident);
Expand Down Expand Up @@ -1713,6 +1721,8 @@ fn find_span_immediately_after_crate_name(
/// entities with that name in all crates. This method allows outputting the
/// results of this search in a programmer-friendly way
crate fn show_candidates(
definitions: &rustc_hir::definitions::Definitions,
session: &Session,
err: &mut DiagnosticBuilder<'_>,
// This is `None` if all placement locations are inside expansions
use_placement_span: Option<Span>,
Expand All @@ -1724,43 +1734,111 @@ crate fn show_candidates(
return;
}

let mut accessible_path_strings: Vec<(String, &str, Option<DefId>)> = Vec::new();
let mut inaccessible_path_strings: Vec<(String, &str, Option<DefId>)> = Vec::new();

candidates.iter().for_each(|c| {
(if c.accessible { &mut accessible_path_strings } else { &mut inaccessible_path_strings })
.push((path_names_to_string(&c.path), c.descr, c.did))
});

// we want consistent results across executions, but candidates are produced
// by iterating through a hash map, so make sure they are ordered:
let mut path_strings: Vec<_> =
candidates.iter().map(|c| path_names_to_string(&c.path)).collect();
for path_strings in [&mut accessible_path_strings, &mut inaccessible_path_strings] {
path_strings.sort_by(|a, b| a.0.cmp(&b.0));
let core_path_strings =
path_strings.drain_filter(|p| p.0.starts_with("core::")).collect::<Vec<_>>();
path_strings.extend(core_path_strings);
path_strings.dedup_by(|a, b| a.0 == b.0);
}

if !accessible_path_strings.is_empty() {
let (determiner, kind) = if accessible_path_strings.len() == 1 {
("this", accessible_path_strings[0].1)
} else {
("one of these", "items")
};

path_strings.sort();
let core_path_strings =
path_strings.drain_filter(|p| p.starts_with("core::")).collect::<Vec<String>>();
path_strings.extend(core_path_strings);
path_strings.dedup();
let instead = if instead { " instead" } else { "" };
let mut msg = format!("consider importing {} {}{}", determiner, kind, instead);

let (determiner, kind) = if candidates.len() == 1 {
("this", candidates[0].descr)
} else {
("one of these", "items")
};

let instead = if instead { " instead" } else { "" };
let mut msg = format!("consider importing {} {}{}", determiner, kind, instead);

if let Some(span) = use_placement_span {
for candidate in &mut path_strings {
// produce an additional newline to separate the new use statement
// from the directly following item.
let additional_newline = if found_use { "" } else { "\n" };
*candidate = format!("use {};\n{}", candidate, additional_newline);
}
if let Some(span) = use_placement_span {
for candidate in &mut accessible_path_strings {
// produce an additional newline to separate the new use statement
// from the directly following item.
let additional_newline = if found_use { "" } else { "\n" };
candidate.0 = format!("use {};\n{}", &candidate.0, additional_newline);
}

err.span_suggestions(span, &msg, path_strings.into_iter(), Applicability::Unspecified);
} else {
msg.push(':');
err.span_suggestions(
span,
&msg,
accessible_path_strings.into_iter().map(|a| a.0),
Applicability::Unspecified,
);
} else {
msg.push(':');

for candidate in path_strings {
msg.push('\n');
msg.push_str(&candidate);
for candidate in accessible_path_strings {
msg.push('\n');
msg.push_str(&candidate.0);
}

err.note(&msg);
}
} else {
assert!(!inaccessible_path_strings.is_empty());

if inaccessible_path_strings.len() == 1 {
let (name, descr, def_id) = &inaccessible_path_strings[0];
let msg = format!("{} `{}` exists but is inaccessible", descr, name);

if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
let span = definitions.def_span(local_def_id);
let span = session.source_map().guess_head_span(span);
let mut multi_span = MultiSpan::from_span(span);
multi_span.push_span_label(span, "not accessible".to_string());
err.span_note(multi_span, &msg);
} else {
err.note(&msg);
}
} else {
let (_, descr_first, _) = &inaccessible_path_strings[0];
let descr = if inaccessible_path_strings
.iter()
.skip(1)
.all(|(_, descr, _)| descr == descr_first)
{
format!("{}", descr_first)
} else {
"item".to_string()
};

let mut msg = format!("these {}s exist but are inaccessible", descr);
let mut has_colon = false;

err.note(&msg);
let mut spans = Vec::new();
for (name, _, def_id) in &inaccessible_path_strings {
if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
let span = definitions.def_span(local_def_id);
let span = session.source_map().guess_head_span(span);
spans.push((name, span));
} else {
if !has_colon {
msg.push(':');
has_colon = true;
}
msg.push('\n');
msg.push_str(name);
}
}

let mut multi_span = MultiSpan::from_spans(spans.iter().map(|(_, sp)| *sp).collect());
for (name, span) in spans {
multi_span.push_span_label(span, format!("`{}`: not accessible", name));
}

err.span_note(multi_span, &msg);
}
}
}
12 changes: 10 additions & 2 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1152,7 +1152,7 @@ impl ResolverAstLowering for Resolver<'_> {
self.legacy_const_generic_args(expr)
}

fn get_partial_res(&mut self, id: NodeId) -> Option<PartialRes> {
fn get_partial_res(&self, id: NodeId) -> Option<PartialRes> {
self.partial_res_map.get(&id).cloned()
}

Expand Down Expand Up @@ -2969,7 +2969,15 @@ impl<'a> Resolver<'a> {
(None, false)
};
if !candidates.is_empty() {
diagnostics::show_candidates(&mut err, span, &candidates, instead, found_use);
diagnostics::show_candidates(
&self.definitions,
self.session,
&mut err,
span,
&candidates,
instead,
found_use,
);
} else if let Some((span, msg, sugg, appl)) = suggestion {
err.span_suggestion(span, msg, sugg, appl);
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2137,7 +2137,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
idx_t
} else {
let base_t = self.structurally_resolved_type(base.span, base_t);
match self.lookup_indexing(expr, base, base_t, idx_t) {
match self.lookup_indexing(expr, base, base_t, idx, idx_t) {
Some((index_ty, element_ty)) => {
// two-phase not needed because index_ty is never mutable
self.demand_coerce(idx, idx_t, index_ty, None, AllowTwoPhase::No);
Expand Down
Loading

0 comments on commit 69eb996

Please sign in to comment.