Skip to content

Commit

Permalink
Rollup merge of #132708 - estebank:const-as-binding, r=Nadrieril
Browse files Browse the repository at this point in the history
Point at `const` definition when used instead of a binding in a `let` statement

Modify `PatKind::InlineConstant` to be `ExpandedConstant` standing in not only for inline `const` blocks but also for `const` items. This allows us to track named `const`s used in patterns when the pattern is a single binding. When we detect that there is a refutable pattern involving a `const` that could have been a binding instead, we point at the `const` item, and suggest renaming. We do this for both `let` bindings and `match` expressions missing a catch-all arm if there's at least one single binding pattern referenced.

After:

```
error[E0005]: refutable pattern in local binding
  --> $DIR/bad-pattern.rs:19:13
   |
LL |     const PAT: u32 = 0;
   |     -------------- missing patterns are not covered because `PAT` is interpreted as a constant pattern, not a new variable
...
LL |         let PAT = v1;
   |             ^^^ pattern `1_u32..=u32::MAX` not covered
   |
   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
   = note: the matched value is of type `u32`
help: introduce a variable instead
   |
LL |         let PAT_var = v1;
   |             ~~~~~~~
```

Before:

```
error[E0005]: refutable pattern in local binding
  --> $DIR/bad-pattern.rs:19:13
   |
LL |         let PAT = v1;
   |             ^^^
   |             |
   |             pattern `1_u32..=u32::MAX` not covered
   |             missing patterns are not covered because `PAT` is interpreted as a constant pattern, not a new variable
   |             help: introduce a variable instead: `PAT_var`
   |
   = note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
   = note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
   = note: the matched value is of type `u32`
```

CC #132582.
  • Loading branch information
matthiaskrgr authored Nov 20, 2024
2 parents 71d3c77 + 29acf8b commit 7fc2b33
Show file tree
Hide file tree
Showing 22 changed files with 365 additions and 94 deletions.
17 changes: 11 additions & 6 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -645,7 +645,7 @@ impl<'tcx> Pat<'tcx> {
| Binding { subpattern: Some(subpattern), .. }
| Deref { subpattern }
| DerefPattern { subpattern, .. }
| InlineConstant { subpattern, .. } => subpattern.walk_(it),
| ExpandedConstant { subpattern, .. } => subpattern.walk_(it),
Leaf { subpatterns } | Variant { subpatterns, .. } => {
subpatterns.iter().for_each(|field| field.pattern.walk_(it))
}
Expand Down Expand Up @@ -788,12 +788,17 @@ pub enum PatKind<'tcx> {
value: mir::Const<'tcx>,
},

/// Inline constant found while lowering a pattern.
InlineConstant {
/// [LocalDefId] of the constant, we need this so that we have a
/// Pattern obtained by converting a constant (inline or named) to its pattern
/// representation using `const_to_pat`.
ExpandedConstant {
/// [DefId] of the constant, we need this so that we have a
/// reference that can be used by unsafety checking to visit nested
/// unevaluated constants.
def: LocalDefId,
/// unevaluated constants and for diagnostics. If the `DefId` doesn't
/// correspond to a local crate, it points at the `const` item.
def_id: DefId,
/// If `false`, then `def_id` points at a `const` item, otherwise it
/// corresponds to a local inline const.
is_inline: bool,
/// If the inline constant is used in a range pattern, this subpattern
/// represents the range (if both ends are inline constants, there will
/// be multiple InlineConstant wrappers).
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ pub fn walk_pat<'thir, 'tcx: 'thir, V: Visitor<'thir, 'tcx>>(
}
}
Constant { value: _ } => {}
InlineConstant { def: _, subpattern } => visitor.visit_pat(subpattern),
ExpandedConstant { def_id: _, is_inline: _, subpattern } => visitor.visit_pat(subpattern),
Range(_) => {}
Slice { prefix, slice, suffix } | Array { prefix, slice, suffix } => {
for subpattern in prefix.iter() {
Expand Down
20 changes: 14 additions & 6 deletions compiler/rustc_mir_build/src/build/custom/parse/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,20 @@ impl<'a, 'tcx> ParseCtxt<'a, 'tcx> {
let mut targets = Vec::new();
for arm in rest {
let arm = &self.thir[*arm];
let PatKind::Constant { value } = arm.pattern.kind else {
return Err(ParseError {
span: arm.pattern.span,
item_description: format!("{:?}", arm.pattern.kind),
expected: "constant pattern".to_string(),
});
let value = match arm.pattern.kind {
PatKind::Constant { value } => value,
PatKind::ExpandedConstant { ref subpattern, def_id: _, is_inline: false }
if let PatKind::Constant { value } = subpattern.kind =>
{
value
}
_ => {
return Err(ParseError {
span: arm.pattern.span,
item_description: format!("{:?}", arm.pattern.kind),
expected: "constant pattern".to_string(),
});
}
};
values.push(value.eval_bits(self.tcx, self.typing_env));
targets.push(self.parse_block(arm.body)?);
Expand Down
8 changes: 6 additions & 2 deletions compiler/rustc_mir_build/src/build/matches/match_pair.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,11 @@ impl<'pat, 'tcx> MatchPairTree<'pat, 'tcx> {
TestCase::Irrefutable { ascription: None, binding }
}

PatKind::InlineConstant { subpattern: ref pattern, def, .. } => {
PatKind::ExpandedConstant { subpattern: ref pattern, def_id: _, is_inline: false } => {
subpairs.push(MatchPairTree::for_pattern(place_builder, pattern, cx));
default_irrefutable()
}
PatKind::ExpandedConstant { subpattern: ref pattern, def_id, is_inline: true } => {
// Apply a type ascription for the inline constant to the value at `match_pair.place`
let ascription = place.map(|source| {
let span = pattern.span;
Expand All @@ -173,7 +177,7 @@ impl<'pat, 'tcx> MatchPairTree<'pat, 'tcx> {
})
.args;
let user_ty = cx.infcx.canonicalize_user_type_annotation(ty::UserType::TypeOf(
def.to_def_id(),
def_id,
ty::UserArgs { args, user_self_ty: None },
));
let annotation = ty::CanonicalUserTypeAnnotation {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
self.visit_primary_bindings(subpattern, subpattern_user_ty, f)
}

PatKind::InlineConstant { ref subpattern, .. } => {
PatKind::ExpandedConstant { ref subpattern, .. } => {
self.visit_primary_bindings(subpattern, pattern_user_ty, f)
}

Expand Down
10 changes: 7 additions & 3 deletions compiler/rustc_mir_build/src/check_unsafety.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
PatKind::Wild |
// these just wrap other patterns, which we recurse on below.
PatKind::Or { .. } |
PatKind::InlineConstant { .. } |
PatKind::ExpandedConstant { .. } |
PatKind::AscribeUserType { .. } |
PatKind::Error(_) => {}
}
Expand Down Expand Up @@ -386,8 +386,12 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> {
visit::walk_pat(self, pat);
self.inside_adt = old_inside_adt;
}
PatKind::InlineConstant { def, .. } => {
self.visit_inner_body(*def);
PatKind::ExpandedConstant { def_id, is_inline, .. } => {
if let Some(def) = def_id.as_local()
&& *is_inline
{
self.visit_inner_body(def);
}
visit::walk_pat(self, pat);
}
_ => {
Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -860,8 +860,10 @@ pub(crate) struct PatternNotCovered<'s, 'tcx> {
pub(crate) uncovered: Uncovered,
#[subdiagnostic]
pub(crate) inform: Option<Inform>,
#[label(mir_build_confused)]
pub(crate) interpreted_as_const: Option<Span>,
#[subdiagnostic]
pub(crate) interpreted_as_const: Option<InterpretedAsConst>,
pub(crate) interpreted_as_const_sugg: Option<InterpretedAsConst>,
#[subdiagnostic]
pub(crate) adt_defined_here: Option<AdtDefinedHere<'tcx>>,
#[note(mir_build_privately_uninhabited)]
Expand Down Expand Up @@ -911,9 +913,9 @@ impl<'tcx> Subdiagnostic for AdtDefinedHere<'tcx> {
#[suggestion(
mir_build_interpreted_as_const,
code = "{variable}_var",
applicability = "maybe-incorrect"
applicability = "maybe-incorrect",
style = "verbose"
)]
#[label(mir_build_confused)]
pub(crate) struct InterpretedAsConst {
#[primary_span]
pub(crate) span: Span,
Expand Down
56 changes: 48 additions & 8 deletions compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -678,8 +678,25 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
let mut let_suggestion = None;
let mut misc_suggestion = None;
let mut interpreted_as_const = None;
let mut interpreted_as_const_sugg = None;

if let PatKind::Constant { .. }
if let PatKind::ExpandedConstant { def_id, is_inline: false, .. }
| PatKind::AscribeUserType {
subpattern:
box Pat { kind: PatKind::ExpandedConstant { def_id, is_inline: false, .. }, .. },
..
} = pat.kind
&& let DefKind::Const = self.tcx.def_kind(def_id)
&& let Ok(snippet) = self.tcx.sess.source_map().span_to_snippet(pat.span)
// We filter out paths with multiple path::segments.
&& snippet.chars().all(|c| c.is_alphanumeric() || c == '_')
{
let span = self.tcx.def_span(def_id);
let variable = self.tcx.item_name(def_id).to_string();
// When we encounter a constant as the binding name, point at the `const` definition.
interpreted_as_const = Some(span);
interpreted_as_const_sugg = Some(InterpretedAsConst { span: pat.span, variable });
} else if let PatKind::Constant { .. }
| PatKind::AscribeUserType {
subpattern: box Pat { kind: PatKind::Constant { .. }, .. },
..
Expand All @@ -692,9 +709,6 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
misc_suggestion = Some(MiscPatternSuggestion::AttemptedIntegerLiteral {
start_span: pat.span.shrink_to_lo(),
});
} else if snippet.chars().all(|c| c.is_alphanumeric() || c == '_') {
interpreted_as_const =
Some(InterpretedAsConst { span: pat.span, variable: snippet });
}
}

Expand Down Expand Up @@ -743,6 +757,7 @@ impl<'p, 'tcx> MatchVisitor<'p, 'tcx> {
uncovered: Uncovered::new(pat.span, &cx, witnesses),
inform,
interpreted_as_const,
interpreted_as_const_sugg,
witness_1_is_privately_uninhabited,
_p: (),
pattern_ty,
Expand Down Expand Up @@ -1112,13 +1127,13 @@ fn report_non_exhaustive_match<'p, 'tcx>(
if ty.is_ptr_sized_integral() {
if ty.inner() == cx.tcx.types.usize {
err.note(format!(
"`{ty}` does not have a fixed maximum value, so half-open ranges are necessary to match \
exhaustively",
"`{ty}` does not have a fixed maximum value, so half-open ranges are \
necessary to match exhaustively",
));
} else if ty.inner() == cx.tcx.types.isize {
err.note(format!(
"`{ty}` does not have fixed minimum and maximum values, so half-open ranges are necessary to match \
exhaustively",
"`{ty}` does not have fixed minimum and maximum values, so half-open \
ranges are necessary to match exhaustively",
));
}
} else if ty.inner() == cx.tcx.types.str_ {
Expand All @@ -1139,6 +1154,31 @@ fn report_non_exhaustive_match<'p, 'tcx>(
}
}

for &arm in arms {
let arm = &thir.arms[arm];
if let PatKind::ExpandedConstant { def_id, is_inline: false, .. } = arm.pattern.kind
&& let Ok(snippet) = cx.tcx.sess.source_map().span_to_snippet(arm.pattern.span)
// We filter out paths with multiple path::segments.
&& snippet.chars().all(|c| c.is_alphanumeric() || c == '_')
{
let const_name = cx.tcx.item_name(def_id);
err.span_label(
arm.pattern.span,
format!(
"this pattern doesn't introduce a new catch-all binding, but rather pattern \
matches against the value of constant `{const_name}`",
),
);
err.span_note(cx.tcx.def_span(def_id), format!("constant `{const_name}` defined here"));
err.span_suggestion_verbose(
arm.pattern.span.shrink_to_hi(),
"if you meant to introduce a binding, use a different name",
"_var".to_string(),
Applicability::MaybeIncorrect,
);
}
}

// Whether we suggest the actual missing patterns or `_`.
let suggest_the_witnesses = witnesses.len() < 4;
let suggested_arm = if suggest_the_witnesses {
Expand Down
42 changes: 30 additions & 12 deletions compiler/rustc_mir_build/src/thir/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,21 +149,30 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
None => Ok((None, None, None)),
Some(expr) => {
let (kind, ascr, inline_const) = match self.lower_lit(expr) {
PatKind::InlineConstant { subpattern, def } => {
(subpattern.kind, None, Some(def))
PatKind::ExpandedConstant { subpattern, def_id, is_inline: true } => {
(subpattern.kind, None, def_id.as_local())
}
PatKind::ExpandedConstant { subpattern, is_inline: false, .. } => {
(subpattern.kind, None, None)
}
PatKind::AscribeUserType { ascription, subpattern: box Pat { kind, .. } } => {
(kind, Some(ascription), None)
}
kind => (kind, None, None),
};
let value = if let PatKind::Constant { value } = kind {
value
} else {
let msg = format!(
"found bad range pattern endpoint `{expr:?}` outside of error recovery"
);
return Err(self.tcx.dcx().span_delayed_bug(expr.span, msg));
let value = match kind {
PatKind::Constant { value } => value,
PatKind::ExpandedConstant { subpattern, .. }
if let PatKind::Constant { value } = subpattern.kind =>
{
value
}
_ => {
let msg = format!(
"found bad range pattern endpoint `{expr:?}` outside of error recovery"
);
return Err(self.tcx.dcx().span_delayed_bug(expr.span, msg));
}
};
Ok((Some(PatRangeBoundary::Finite(value)), ascr, inline_const))
}
Expand Down Expand Up @@ -288,7 +297,11 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
};
}
for def in [lo_inline, hi_inline].into_iter().flatten() {
kind = PatKind::InlineConstant { def, subpattern: Box::new(Pat { span, ty, kind }) };
kind = PatKind::ExpandedConstant {
def_id: def.to_def_id(),
is_inline: true,
subpattern: Box::new(Pat { span, ty, kind }),
};
}
Ok(kind)
}
Expand Down Expand Up @@ -562,7 +575,12 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {

let args = self.typeck_results.node_args(id);
let c = ty::Const::new_unevaluated(self.tcx, ty::UnevaluatedConst { def: def_id, args });
let pattern = self.const_to_pat(c, ty, id, span);
let subpattern = self.const_to_pat(c, ty, id, span);
let pattern = Box::new(Pat {
span,
ty,
kind: PatKind::ExpandedConstant { subpattern, def_id, is_inline: false },
});

if !is_associated_const {
return pattern;
Expand Down Expand Up @@ -637,7 +655,7 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {

let ct = ty::UnevaluatedConst { def: def_id.to_def_id(), args };
let subpattern = self.const_to_pat(ty::Const::new_unevaluated(self.tcx, ct), ty, id, span);
PatKind::InlineConstant { subpattern, def: def_id }
PatKind::ExpandedConstant { subpattern, def_id: def_id.to_def_id(), is_inline: true }
}

/// Converts literals, paths and negation of literals to patterns.
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_mir_build/src/thir/print.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,9 +707,10 @@ impl<'a, 'tcx> ThirPrinter<'a, 'tcx> {
print_indented!(self, format!("value: {:?}", value), depth_lvl + 2);
print_indented!(self, "}", depth_lvl + 1);
}
PatKind::InlineConstant { def, subpattern } => {
print_indented!(self, "InlineConstant {", depth_lvl + 1);
print_indented!(self, format!("def: {:?}", def), depth_lvl + 2);
PatKind::ExpandedConstant { def_id, is_inline, subpattern } => {
print_indented!(self, "ExpandedConstant {", depth_lvl + 1);
print_indented!(self, format!("def_id: {def_id:?}"), depth_lvl + 2);
print_indented!(self, format!("is_inline: {is_inline:?}"), depth_lvl + 2);
print_indented!(self, "subpattern:", depth_lvl + 2);
self.print_pat(subpattern, depth_lvl + 2);
print_indented!(self, "}", depth_lvl + 1);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_pattern_analysis/src/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ impl<'p, 'tcx: 'p> RustcPatCtxt<'p, 'tcx> {
let fields: Vec<_>;
match &pat.kind {
PatKind::AscribeUserType { subpattern, .. }
| PatKind::InlineConstant { subpattern, .. } => return self.lower_pat(subpattern),
| PatKind::ExpandedConstant { subpattern, .. } => return self.lower_pat(subpattern),
PatKind::Binding { subpattern: Some(subpat), .. } => return self.lower_pat(subpat),
PatKind::Binding { subpattern: None, .. } | PatKind::Wild => {
ctor = Wildcard;
Expand Down
13 changes: 8 additions & 5 deletions tests/ui/closures/2229_closure_analysis/bad-pattern.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -97,16 +97,19 @@ LL | if let Refutable::A = v3 { todo!() };
error[E0005]: refutable pattern in local binding
--> $DIR/bad-pattern.rs:19:13
|
LL | const PAT: u32 = 0;
| -------------- missing patterns are not covered because `PAT` is interpreted as a constant pattern, not a new variable
...
LL | let PAT = v1;
| ^^^
| |
| pattern `1_u32..=u32::MAX` not covered
| missing patterns are not covered because `PAT` is interpreted as a constant pattern, not a new variable
| help: introduce a variable instead: `PAT_var`
| ^^^ pattern `1_u32..=u32::MAX` not covered
|
= note: `let` bindings require an "irrefutable pattern", like a `struct` or an `enum` with only one variant
= note: for more information, visit https://doc.rust-lang.org/book/ch18-02-refutability.html
= note: the matched value is of type `u32`
help: introduce a variable instead
|
LL | let PAT_var = v1;
| ~~~~~~~

error: aborting due to 7 previous errors

Expand Down
Loading

0 comments on commit 7fc2b33

Please sign in to comment.