bjoernager/rust
bjoernager/rust
1
Fork 0
Code Issues Pull requests Activity

Keep track of patterns that could have introduced a binding, but didn't

Browse source 
When we recover from a pattern parse error, or a pattern uses `..`, we keep track of that and affect resolution error for missing bindings that could have been provided by that pattern. We differentiate between `..` and parse recovery. We silence resolution errors likely caused by the pattern parse error.

```
error[E0425]: cannot find value `title` in this scope
  --> $DIR/struct-pattern-with-missing-fields-resolve-error.rs:19:30
   |
LL |         println!("[{}]({})", title, url);
   |                              ^^^^^ not found in this scope
   |
note: `Website` has a field `title` which could have been included in this pattern, but it wasn't
  --> $DIR/struct-pattern-with-missing-fields-resolve-error.rs:17:12
   |
LL | / struct Website {
LL | |     url: String,
LL | |     title: Option<String> ,
   | |     ----- defined here
LL | | }
   | |_-
...
LL |       if let Website { url, .. } = website {
   |              ^^^^^^^^^^^^^^^^^^^ this pattern doesn't include `title`, which is available in `Website`
```

Fix #74863.
This commit is contained in:
Esteban Küber 2024-12-13 21:51:33 +00:00
parent 21fe748be1
commit 0f82cfffda
8 changed files with 158 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

2
compiler/rustc_ast/src/ast.rs
View file

@ -859,6 +859,8 @@ pub enum PatKind {
pub enum PatFieldsRest {
/// `module::StructName { field, ..}`
Rest,
/// `module::StructName { field, syntax error }`
Recovered(ErrorGuaranteed),
/// `module::StructName { field }`
None,
}

9
compiler/rustc_ast_lowering/src/pat.rs
View file

@ -92,7 +92,14 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
span: self.lower_span(f.span),
}
}));
break hir::PatKind::Struct(qpath, fs, *etc == ast::PatFieldsRest::Rest);
break hir::PatKind::Struct(
qpath,
fs,
matches!(
etc,
ast::PatFieldsRest::Rest | ast::PatFieldsRest::Recovered(_)
),
);
}
PatKind::Tuple(pats) => {
let (pats, ddpos) = self.lower_pat_tuple(pats, "tuple");

5
compiler/rustc_ast_pretty/src/pprust/state.rs
View file

@ -1653,11 +1653,14 @@ impl<'a> State<'a> {
},
|f| f.pat.span,
);
if *etc == ast::PatFieldsRest::Rest {
if let ast::PatFieldsRest::Rest | ast::PatFieldsRest::Recovered(_) = etc {
if !fields.is_empty() {
self.word_space(",");
}
self.word("..");
if let ast::PatFieldsRest::Recovered(_) = etc {
self.word("/* recovered parse error */");
}
}
if !empty {
self.space();

4
compiler/rustc_parse/src/parser/pat.rs
View file

@ -1371,10 +1371,10 @@ impl<'a> Parser<'a> {
self.bump();
let (fields, etc) = self.parse_pat_fields().unwrap_or_else(|mut e| {
e.span_label(path.span, "while parsing the fields for this pattern");
e.emit();
let guar = e.emit();
self.recover_stmt();
// When recovering, pretend we had `Foo { .. }`, to avoid cascading errors.
(ThinVec::new(), PatFieldsRest::Rest)
(ThinVec::new(), PatFieldsRest::Recovered(guar))
});
self.bump();
Ok(PatKind::Struct(qself, path, fields, etc))

33
compiler/rustc_resolve/src/late.rs
View file

@ -264,12 +264,17 @@ impl RibKind<'_> {
#[derive(Debug)]
pub(crate) struct Rib<'ra, R = Res> {
pub bindings: IdentMap<R>,
pub patterns_with_skipped_bindings: FxHashMap<DefId, Vec<(Span, bool /* recovered error */)>>,
pub kind: RibKind<'ra>,
}
impl<'ra, R> Rib<'ra, R> {
fn new(kind: RibKind<'ra>) -> Rib<'ra, R> {
Rib { bindings: Default::default(), kind }
Rib {
bindings: Default::default(),
patterns_with_skipped_bindings: Default::default(),
kind,
}
}
}
@ -3753,6 +3758,7 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
/// When a whole or-pattern has been dealt with, the thing happens.
///
/// See the implementation and `fresh_binding` for more details.
#[tracing::instrument(skip(self, bindings), level = "debug")]
fn resolve_pattern_inner(
&mut self,
pat: &Pat,
@ -3761,7 +3767,6 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
) {
// Visit all direct subpatterns of this pattern.
pat.walk(&mut |pat| {
debug!("resolve_pattern pat={:?} node={:?}", pat, pat.kind);
match pat.kind {
PatKind::Ident(bmode, ident, ref sub) => {
// First try to resolve the identifier as some existing entity,
@ -3787,8 +3792,9 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
PatKind::Path(ref qself, ref path) => {
self.smart_resolve_path(pat.id, qself, path, PathSource::Pat);
}
PatKind::Struct(ref qself, ref path, ..) => {
PatKind::Struct(ref qself, ref path, ref _fields, ref rest) => {
self.smart_resolve_path(pat.id, qself, path, PathSource::Struct);
self.record_patterns_with_skipped_bindings(pat, rest);
}
PatKind::Or(ref ps) => {
// Add a new set of bindings to the stack. `Or` here records that when a
@ -3821,6 +3827,27 @@ impl<'a, 'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
});
}
fn record_patterns_with_skipped_bindings(&mut self, pat: &Pat, rest: &ast::PatFieldsRest) {
match rest {
ast::PatFieldsRest::Rest | ast::PatFieldsRest::Recovered(_) => {
// Record that the pattern doesn't introduce all the bindings it could.
if let Some(partial_res) = self.r.partial_res_map.get(&pat.id)
&& let Some(res) = partial_res.full_res()
&& let Some(def_id) = res.opt_def_id()
{
self.ribs[ValueNS]
.last_mut()
.unwrap()
.patterns_with_skipped_bindings
.entry(def_id)
.or_default()
.push((pat.span, matches!(rest, ast::PatFieldsRest::Recovered(_))));
}
}
ast::PatFieldsRest::None => {}
}
}
fn fresh_binding(
&mut self,
ident: Ident,

60
compiler/rustc_resolve/src/late/diagnostics.rs
View file

@ -430,6 +430,7 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
let mut err = self.r.dcx().struct_span_err(base_error.span, base_error.msg.clone());
err.code(code);
self.detect_missing_binding_available_from_pattern(&mut err, path, following_seg);
self.suggest_at_operator_in_slice_pat_with_range(&mut err, path);
self.suggest_swapping_misplaced_self_ty_and_trait(&mut err, source, res, base_error.span);
@ -1120,6 +1121,65 @@ impl<'ast, 'ra: 'ast, 'tcx> LateResolutionVisitor<'_, 'ast, 'ra, 'tcx> {
true
}
fn detect_missing_binding_available_from_pattern(
&mut self,
err: &mut Diag<'_>,
path: &[Segment],
following_seg: Option<&Segment>,
) {
let [segment] = path else { return };
let None = following_seg else { return };
'outer: for rib in self.ribs[ValueNS].iter().rev() {
for (def_id, spans) in &rib.patterns_with_skipped_bindings {
if let Some(fields) = self.r.field_idents(*def_id) {
for field in fields {
if field.name == segment.ident.name {
if spans.iter().all(|(_, was_recovered)| *was_recovered) {
// This resolution error will likely be fixed by fixing a
// syntax error in a pattern, so it is irrelevant to the user.
let multispan: MultiSpan =
spans.iter().map(|(s, _)| *s).collect::<Vec<_>>().into();
err.span_note(
multispan,
"this pattern had a recovered parse error which likely \
lost the expected fields",
);
err.downgrade_to_delayed_bug();
}
let mut multispan: MultiSpan = spans
.iter()
.filter(|(_, was_recovered)| !was_recovered)
.map(|(sp, _)| *sp)
.collect::<Vec<_>>()
.into();
let def_span = self.r.def_span(*def_id);
let ty = self.r.tcx.item_name(*def_id);
multispan.push_span_label(def_span, String::new());
multispan.push_span_label(field.span, "defined here".to_string());
for (span, _) in spans.iter().filter(|(_, r)| !r) {
multispan.push_span_label(
*span,
format!(
"this pattern doesn't include `{field}`, which is \
available in `{ty}`",
),
);
}
err.span_note(
multispan,
format!(
"`{ty}` has a field `{field}` which could have been included \
in this pattern, but it wasn't",
),
);
break 'outer;
}
}
}
}
}
}
fn suggest_at_operator_in_slice_pat_with_range(
&mut self,
err: &mut Diag<'_>,

22
tests/ui/pattern/struct-pattern-with-missing-fields-resolve-error.rs Normal file
View file

@ -0,0 +1,22 @@
struct Website {
url: String,
title: Option<String> ,//~ NOTE defined here
}
fn main() {
let website = Website {
url: "http://www.example.com".into(),
title: Some("Example Domain".into()),
};
if let Website { url, Some(title) } = website { //~ ERROR expected `,`
//~^ NOTE while parsing the fields for this pattern
println!("[{}]({})", title, url); // we hide the errors for `title` and `url`
}
if let Website { url, .. } = website { //~ NOTE `Website` has a field `title`
//~^ NOTE this pattern
println!("[{}]({})", title, url); //~ ERROR cannot find value `title` in this scope
//~^ NOTE not found in this scope
}
}

30
tests/ui/pattern/struct-pattern-with-missing-fields-resolve-error.stderr Normal file
View file

@ -0,0 +1,30 @@
error: expected `,`
--> $DIR/struct-pattern-with-missing-fields-resolve-error.rs:12:31
|
LL | if let Website { url, Some(title) } = website {
| ------- ^
| |
| while parsing the fields for this pattern
error[E0425]: cannot find value `title` in this scope
--> $DIR/struct-pattern-with-missing-fields-resolve-error.rs:19:30
|
LL | println!("[{}]({})", title, url);
| ^^^^^ not found in this scope
|
note: `Website` has a field `title` which could have been included in this pattern, but it wasn't
--> $DIR/struct-pattern-with-missing-fields-resolve-error.rs:17:12
|
LL | / struct Website {
LL | | url: String,
LL | | title: Option<String> ,
| | ----- defined here
LL | | }
| |_-
...
LL | if let Website { url, .. } = website {
| ^^^^^^^^^^^^^^^^^^^ this pattern doesn't include `title`, which is available in `Website`
error: aborting due to 2 previous errors
For more information about this error, try `rustc --explain E0425`.