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

Remove label/lifetime shadowing warnings #96296

Merged
merged 9 commits into from
Jun 3, 2022
Merged
Prev Previous commit
Next Next commit
Reuse resolve_label to check lifetime shadowing.
  • Loading branch information
cjgillot committed Jun 3, 2022
commit 86bd99060c5f2b60e27c6afa5ad9c904333cb746
53 changes: 20 additions & 33 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1548,13 +1548,9 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {

/// Searches the current set of local scopes for labels. Returns the `NodeId` of the resolved
/// label and reports an error if the label is not found or is unreachable.
fn resolve_label(&mut self, mut label: Ident) -> Option<NodeId> {
fn resolve_label(&mut self, mut label: Ident) -> Result<(NodeId, Span), ResolutionError<'a>> {
let mut suggestion = None;

// Preserve the original span so that errors contain "in this macro invocation"
// information.
let original_span = label.span;

for i in (0..self.label_ribs.len()).rev() {
let rib = &self.label_ribs[i];

Expand All @@ -1570,18 +1566,13 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
if let Some((ident, id)) = rib.bindings.get_key_value(&ident) {
let definition_span = ident.span;
return if self.is_label_valid_from_rib(i) {
Some(*id)
Ok((*id, definition_span))
} else {
self.report_error(
original_span,
ResolutionError::UnreachableLabel {
name: label.name,
definition_span,
suggestion,
},
);

None
Err(ResolutionError::UnreachableLabel {
name: label.name,
definition_span,
suggestion,
})
};
}

Expand All @@ -1590,11 +1581,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
suggestion = suggestion.or_else(|| self.suggestion_for_label_in_rib(i, label));
}

self.report_error(
original_span,
ResolutionError::UndeclaredLabel { name: label.name, suggestion },
);
None
Err(ResolutionError::UndeclaredLabel { name: label.name, suggestion })
}

/// Determine whether or not a label from the `rib_index`th label rib is reachable.
Expand Down Expand Up @@ -3152,17 +3139,12 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
self.diagnostic_metadata.unused_labels.insert(id, label.ident.span);
}

let ident = label.ident.normalize_to_macro_rules();
for rib in self.label_ribs.iter_mut().rev() {
if let Some((&orig_ident, _)) = rib.bindings.get_key_value(&ident) {
diagnostics::signal_label_shadowing(self.r.session, orig_ident, label.ident)
}
if rib.kind.is_label_barrier() {
break;
}
if let Ok((_, orig_span)) = self.resolve_label(label.ident) {
diagnostics::signal_label_shadowing(self.r.session, orig_span, label.ident)
}

self.with_label_rib(NormalRibKind, |this| {
let ident = label.ident.normalize_to_macro_rules();
this.label_ribs.last_mut().unwrap().bindings.insert(ident, id);
f(this);
});
Expand Down Expand Up @@ -3266,10 +3248,15 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
}

ExprKind::Break(Some(label), _) | ExprKind::Continue(Some(label)) => {
if let Some(node_id) = self.resolve_label(label.ident) {
// Since this res is a label, it is never read.
self.r.label_res_map.insert(expr.id, node_id);
self.diagnostic_metadata.unused_labels.remove(&node_id);
match self.resolve_label(label.ident) {
Ok((node_id, _)) => {
// Since this res is a label, it is never read.
self.r.label_res_map.insert(expr.id, node_id);
self.diagnostic_metadata.unused_labels.remove(&node_id);
}
Err(error) => {
self.report_error(label.ident.span, error);
}
}

// visit `break` argument if any
Expand Down
16 changes: 9 additions & 7 deletions compiler/rustc_resolve/src/late/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2066,15 +2066,17 @@ pub fn signal_lifetime_shadowing(
err.emit();
}

/// Shadowing involving a label is only a warning, due to issues with
/// labels and lifetimes not being macro-hygienic.
pub fn signal_label_shadowing(sess: &Session, orig: Ident, shadower: Ident) {
/// Shadowing involving a label is only a warning for historical reasons.
//FIXME: make this a proper lint.
pub fn signal_label_shadowing(sess: &Session, orig: Span, shadower: Ident) {
let name = shadower.name;
let shadower = shadower.span;
let mut err = sess.struct_span_warn(
shadower.span,
&format!("label name `{}` shadows a label name that is already in scope", orig.name),
shadower,
&format!("label name `{}` shadows a label name that is already in scope", name),
);
err.span_label(orig.span, "first declared here");
err.span_label(shadower.span, format!("label `{}` already in scope", orig.name));
err.span_label(orig, "first declared here");
err.span_label(shadower, format!("label `{}` already in scope", name));
err.emit();
}

Expand Down