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

Point at const definition when used instead of a binding in a let statement #132708

Merged
merged 10 commits into from
Nov 21, 2024
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.param_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 } => {
Comment on lines +165 to +169
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here...

// 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);
}
Comment on lines +389 to +394
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...and here are the two places where is_inline has any effect in the constant evaluation. @compiler-errors can you take a look to see if this is a reasonable level of complexity? We can also add comments explaining why is_inline (or rather, why we don't want to evaluate the unsafety of const items). The current approach has the nice property of not making the types any larger than they were.

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 @@ -668,8 +668,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)
Copy link
Member

@compiler-errors compiler-errors Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you read my mind with the DefId -- but we can take it further and use the tcx.item_name here to get the name of the constant, rather than using snippets. This will regress reexported constants, but is more resilient IMO and is worth the change.

Copy link
Member

@compiler-errors compiler-errors Nov 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specifically, this pat name will not possibly be tied to arbitrary source code, in pat_macro!(/* uwu */).

// 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 { .. }, .. },
..
Nadrieril marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -682,9 +699,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 @@ -733,6 +747,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 @@ -1102,13 +1117,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 @@ -1129,6 +1144,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 @@ -453,7 +453,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
Loading