Skip to content

Commit

Permalink
Auto merge of #96515 - lcnr:user-types-in-pat, r=nikomatsakis
Browse files Browse the repository at this point in the history
correctly deal with user type ascriptions in pat

supersedes #93856

`thir::PatKind::AscribeUserType` previously resulted in `CanonicalUserTypeAnnotations` where the inferred type already had a subtyping relation according to `variance` to the `user_ty`.

The bug can pretty much be summarized as follows:

- during mir building
  - `user_ty -> inferred_ty`: considers variance
  - `StatementKind::AscribeUserType`: `inferred_ty` is the type of the place, so no variance needed
- during mir borrowck
  - `user_ty -> inferred_ty`: does not consider variance
  - `StatementKind::AscribeUserType`: applies variance

This mostly worked fine. The lifetimes in `inferred_ty` were only bound by its relation to `user_ty` and to the `place` of `StatementKind::AscribeUserType`, so it doesn't matter where exactly the subtyping happens.

It does however matter when having higher ranked subtying. At this point the place where the subtyping happens is forced, causing this mismatch between building and borrowck to result in unintended errors.

cc #96514 which is pretty much the same issue

r? `@nikomatsakis`
  • Loading branch information
bors committed May 21, 2022
2 parents 9257f5a + 7637008 commit e52e711
Show file tree
Hide file tree
Showing 21 changed files with 304 additions and 189 deletions.
5 changes: 3 additions & 2 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1007,10 +1007,11 @@ fn write_user_type_annotations(
for (index, annotation) in body.user_type_annotations.iter_enumerated() {
writeln!(
w,
"| {:?}: {:?} at {}",
"| {:?}: user_ty: {:?}, span: {}, inferred_ty: {:?}",
index.index(),
annotation.user_ty,
tcx.sess.source_map().span_to_embeddable_string(annotation.span)
tcx.sess.source_map().span_to_embeddable_string(annotation.span),
annotation.inferred_ty,
)?;
}
if !body.user_type_annotations.is_empty() {
Expand Down
50 changes: 9 additions & 41 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,11 @@ use rustc_index::vec::IndexVec;
use rustc_middle::infer::canonical::Canonical;
use rustc_middle::middle::region;
use rustc_middle::mir::interpret::AllocId;
use rustc_middle::mir::{
self, BinOp, BorrowKind, FakeReadCause, Field, Mutability, UnOp, UserTypeProjection,
};
use rustc_middle::mir::{self, BinOp, BorrowKind, FakeReadCause, Field, Mutability, UnOp};
use rustc_middle::ty::adjustment::PointerCast;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::CanonicalUserTypeAnnotation;
use rustc_middle::ty::{self, AdtDef, Ty, UpvarSubsts, UserType};
use rustc_middle::ty::{
CanonicalUserType, CanonicalUserTypeAnnotation, CanonicalUserTypeAnnotations,
};
use rustc_span::{Span, Symbol, DUMMY_SP};
use rustc_target::abi::VariantIdx;
use rustc_target::asm::InlineAsmRegOrRegClass;
Expand Down Expand Up @@ -540,13 +536,13 @@ pub enum BindingMode {
ByRef(BorrowKind),
}

#[derive(Clone, Debug, PartialEq, HashStable)]
#[derive(Clone, Debug, HashStable)]
pub struct FieldPat<'tcx> {
pub field: Field,
pub pattern: Pat<'tcx>,
}

#[derive(Clone, Debug, PartialEq, HashStable)]
#[derive(Clone, Debug, HashStable)]
pub struct Pat<'tcx> {
pub ty: Ty<'tcx>,
pub span: Span,
Expand All @@ -559,37 +555,10 @@ impl<'tcx> Pat<'tcx> {
}
}

#[derive(Copy, Clone, Debug, PartialEq, HashStable)]
pub struct PatTyProj<'tcx> {
pub user_ty: CanonicalUserType<'tcx>,
}

impl<'tcx> PatTyProj<'tcx> {
pub fn from_user_type(user_annotation: CanonicalUserType<'tcx>) -> Self {
Self { user_ty: user_annotation }
}

pub fn user_ty(
self,
annotations: &mut CanonicalUserTypeAnnotations<'tcx>,
inferred_ty: Ty<'tcx>,
span: Span,
) -> UserTypeProjection {
UserTypeProjection {
base: annotations.push(CanonicalUserTypeAnnotation {
span,
user_ty: self.user_ty,
inferred_ty,
}),
projs: Vec::new(),
}
}
}

#[derive(Copy, Clone, Debug, PartialEq, HashStable)]
#[derive(Clone, Debug, HashStable)]
pub struct Ascription<'tcx> {
pub user_ty: PatTyProj<'tcx>,
/// Variance to use when relating the type `user_ty` to the **type of the value being
pub annotation: CanonicalUserTypeAnnotation<'tcx>,
/// Variance to use when relating the `user_ty` to the **type of the value being
/// matched**. Typically, this is `Variance::Covariant`, since the value being matched must
/// have a type that is some subtype of the ascribed type.
///
Expand All @@ -608,12 +577,11 @@ pub struct Ascription<'tcx> {
/// probably be checking for a `PartialEq` impl instead, but this preserves the behavior
/// of the old type-check for now. See #57280 for details.
pub variance: ty::Variance,
pub user_ty_span: Span,
}

#[derive(Clone, Debug, PartialEq, HashStable)]
#[derive(Clone, Debug, HashStable)]
pub enum PatKind<'tcx> {
/// A wildward pattern: `_`.
/// A wildcard pattern: `_`.
Wild,

AscribeUserType {
Expand Down
59 changes: 22 additions & 37 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
},
..
},
ascription:
thir::Ascription { user_ty: pat_ascription_ty, variance: _, user_ty_span },
ascription: thir::Ascription { annotation, variance: _ },
} => {
let place =
self.storage_live_binding(block, var, irrefutable_pat.span, OutsideGuard, true);
Expand All @@ -535,18 +534,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
let cause_let = FakeReadCause::ForLet(None);
self.cfg.push_fake_read(block, pattern_source_info, cause_let, place);

let ty_source_info = self.source_info(user_ty_span);
let user_ty = pat_ascription_ty.user_ty(
&mut self.canonical_user_type_annotations,
place.ty(&self.local_decls, self.tcx).ty,
ty_source_info.span,
);
let ty_source_info = self.source_info(annotation.span);

let base = self.canonical_user_type_annotations.push(annotation);
self.cfg.push(
block,
Statement {
source_info: ty_source_info,
kind: StatementKind::AscribeUserType(
Box::new((place, user_ty)),
Box::new((place, UserTypeProjection { base, projs: Vec::new() })),
// We always use invariant as the variance here. This is because the
// variance field from the ascription refers to the variance to use
// when applying the type to the value being matched, but this
Expand Down Expand Up @@ -789,7 +785,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

PatKind::AscribeUserType {
ref subpattern,
ascription: thir::Ascription { ref user_ty, user_ty_span, variance: _ },
ascription: thir::Ascription { ref annotation, variance: _ },
} => {
// This corresponds to something like
//
Expand All @@ -799,16 +795,13 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
//
// Note that the variance doesn't apply here, as we are tracking the effect
// of `user_ty` on any bindings contained with subpattern.
let annotation = CanonicalUserTypeAnnotation {
span: user_ty_span,
user_ty: user_ty.user_ty,
inferred_ty: subpattern.ty,
};

let projection = UserTypeProjection {
base: self.canonical_user_type_annotations.push(annotation),
base: self.canonical_user_type_annotations.push(annotation.clone()),
projs: Vec::new(),
};
let subpattern_user_ty = pattern_user_ty.push_projection(&projection, user_ty_span);
let subpattern_user_ty =
pattern_user_ty.push_projection(&projection, annotation.span);
self.visit_primary_bindings(subpattern, subpattern_user_ty, f)
}

Expand Down Expand Up @@ -932,9 +925,8 @@ struct Binding<'tcx> {
/// influence region inference.
#[derive(Clone, Debug)]
struct Ascription<'tcx> {
span: Span,
source: Place<'tcx>,
user_ty: PatTyProj<'tcx>,
annotation: CanonicalUserTypeAnnotation<'tcx>,
variance: ty::Variance,
}

Expand Down Expand Up @@ -1863,7 +1855,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
parent_bindings
.iter()
.flat_map(|(_, ascriptions)| ascriptions)
.chain(&candidate.ascriptions),
.cloned()
.chain(candidate.ascriptions),
);

// rust-lang/rust#27282: The `autoref` business deserves some
Expand Down Expand Up @@ -2067,32 +2060,24 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

/// Append `AscribeUserType` statements onto the end of `block`
/// for each ascription
fn ascribe_types<'b>(
fn ascribe_types(
&mut self,
block: BasicBlock,
ascriptions: impl IntoIterator<Item = &'b Ascription<'tcx>>,
) where
'tcx: 'b,
{
ascriptions: impl IntoIterator<Item = Ascription<'tcx>>,
) {
for ascription in ascriptions {
let source_info = self.source_info(ascription.span);
let source_info = self.source_info(ascription.annotation.span);

debug!(
"adding user ascription at span {:?} of place {:?} and {:?}",
source_info.span, ascription.source, ascription.user_ty,
);

let user_ty = ascription.user_ty.user_ty(
&mut self.canonical_user_type_annotations,
ascription.source.ty(&self.local_decls, self.tcx).ty,
source_info.span,
);
let base = self.canonical_user_type_annotations.push(ascription.annotation);
self.cfg.push(
block,
Statement {
source_info,
kind: StatementKind::AscribeUserType(
Box::new((ascription.source, user_ty)),
Box::new((
ascription.source,
UserTypeProjection { base, projs: Vec::new() },
)),
ascription.variance,
),
},
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_mir_build/src/build/matches/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,15 +152,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
match *match_pair.pattern.kind {
PatKind::AscribeUserType {
ref subpattern,
ascription: thir::Ascription { variance, user_ty, user_ty_span },
ascription: thir::Ascription { ref annotation, variance },
} => {
// Apply the type ascription to the value at `match_pair.place`, which is the
if let Ok(place_resolved) =
match_pair.place.clone().try_upvars_resolved(self.tcx, self.typeck_results)
{
candidate.ascriptions.push(Ascription {
span: user_ty_span,
user_ty,
annotation: annotation.clone(),
source: place_resolved.into_place(self.tcx, self.typeck_results),
variance,
});
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_mir_build/src/thir/cx/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use rustc_middle::thir::*;
use rustc_middle::ty;

use rustc_index::vec::Idx;
use rustc_middle::ty::CanonicalUserTypeAnnotation;

impl<'tcx> Cx<'tcx> {
pub(crate) fn mirror_block(&mut self, block: &'tcx hir::Block<'tcx>) -> Block {
Expand Down Expand Up @@ -80,13 +81,17 @@ impl<'tcx> Cx<'tcx> {
self.typeck_results.user_provided_types().get(ty.hir_id)
{
debug!("mirror_stmts: user_ty={:?}", user_ty);
let annotation = CanonicalUserTypeAnnotation {
user_ty,
span: ty.span,
inferred_ty: self.typeck_results.node_type(ty.hir_id),
};
pattern = Pat {
ty: pattern.ty,
span: pattern.span,
kind: Box::new(PatKind::AscribeUserType {
ascription: Ascription {
user_ty: PatTyProj::from_user_type(user_ty),
user_ty_span: ty.span,
annotation,
variance: ty::Variance::Covariant,
},
subpattern: pattern,
Expand Down
42 changes: 22 additions & 20 deletions compiler/rustc_mir_build/src/thir/pattern/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ use rustc_middle::mir::interpret::{get_slice_bytes, ConstValue};
use rustc_middle::mir::interpret::{ErrorHandled, LitToConstError, LitToConstInput};
use rustc_middle::mir::{self, UserTypeProjection};
use rustc_middle::mir::{BorrowKind, Field, Mutability};
use rustc_middle::thir::{Ascription, BindingMode, FieldPat, Pat, PatKind, PatRange, PatTyProj};
use rustc_middle::thir::{Ascription, BindingMode, FieldPat, Pat, PatKind, PatRange};
use rustc_middle::ty::subst::{GenericArg, SubstsRef};
use rustc_middle::ty::CanonicalUserTypeAnnotation;
use rustc_middle::ty::{self, AdtDef, ConstKind, DefIdTree, Region, Ty, TyCtxt, UserType};
use rustc_span::{Span, Symbol};

Expand Down Expand Up @@ -227,7 +228,8 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
for end in &[lo, hi] {
if let Some((_, Some(ascription))) = end {
let subpattern = Pat { span: pat.span, ty, kind: Box::new(kind) };
kind = PatKind::AscribeUserType { ascription: *ascription, subpattern };
kind =
PatKind::AscribeUserType { ascription: ascription.clone(), subpattern };
}
}

Expand Down Expand Up @@ -432,13 +434,14 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {

if let Some(user_ty) = self.user_substs_applied_to_ty_of_hir_id(hir_id) {
debug!("lower_variant_or_leaf: kind={:?} user_ty={:?} span={:?}", kind, user_ty, span);
let annotation = CanonicalUserTypeAnnotation {
user_ty,
span,
inferred_ty: self.typeck_results.node_type(hir_id),
};
kind = PatKind::AscribeUserType {
subpattern: Pat { span, ty, kind: Box::new(kind) },
ascription: Ascription {
user_ty: PatTyProj::from_user_type(user_ty),
user_ty_span: span,
variance: ty::Variance::Covariant,
},
ascription: Ascription { annotation, variance: ty::Variance::Covariant },
};
}

Expand Down Expand Up @@ -499,18 +502,21 @@ impl<'a, 'tcx> PatCtxt<'a, 'tcx> {
}

let user_provided_types = self.typeck_results().user_provided_types();
if let Some(u_ty) = user_provided_types.get(id) {
let user_ty = PatTyProj::from_user_type(*u_ty);
if let Some(&user_ty) = user_provided_types.get(id) {
let annotation = CanonicalUserTypeAnnotation {
user_ty,
span,
inferred_ty: self.typeck_results().node_type(id),
};
Pat {
span,
kind: Box::new(PatKind::AscribeUserType {
subpattern: pattern,
ascription: Ascription {
annotation,
/// Note that use `Contravariant` here. See the
/// `variance` field documentation for details.
variance: ty::Variance::Contravariant,
user_ty,
user_ty_span: span,
},
}),
ty: const_.ty(),
Expand Down Expand Up @@ -645,7 +651,7 @@ impl<'tcx, T: PatternFoldable<'tcx>> PatternFoldable<'tcx> for Option<T> {
}
}

macro_rules! CloneImpls {
macro_rules! ClonePatternFoldableImpls {
(<$lt_tcx:tt> $($ty:ty),+) => {
$(
impl<$lt_tcx> PatternFoldable<$lt_tcx> for $ty {
Expand All @@ -657,11 +663,11 @@ macro_rules! CloneImpls {
}
}

CloneImpls! { <'tcx>
ClonePatternFoldableImpls! { <'tcx>
Span, Field, Mutability, Symbol, hir::HirId, usize, ty::Const<'tcx>,
Region<'tcx>, Ty<'tcx>, BindingMode, AdtDef<'tcx>,
SubstsRef<'tcx>, &'tcx GenericArg<'tcx>, UserType<'tcx>,
UserTypeProjection, PatTyProj<'tcx>
UserTypeProjection, CanonicalUserTypeAnnotation<'tcx>
}

impl<'tcx> PatternFoldable<'tcx> for FieldPat<'tcx> {
Expand Down Expand Up @@ -694,14 +700,10 @@ impl<'tcx> PatternFoldable<'tcx> for PatKind<'tcx> {
PatKind::Wild => PatKind::Wild,
PatKind::AscribeUserType {
ref subpattern,
ascription: Ascription { variance, ref user_ty, user_ty_span },
ascription: Ascription { ref annotation, variance },
} => PatKind::AscribeUserType {
subpattern: subpattern.fold_with(folder),
ascription: Ascription {
user_ty: user_ty.fold_with(folder),
variance,
user_ty_span,
},
ascription: Ascription { annotation: annotation.fold_with(folder), variance },
},
PatKind::Binding { mutability, name, mode, var, ty, ref subpattern, is_primary } => {
PatKind::Binding {
Expand Down
Loading

0 comments on commit e52e711

Please sign in to comment.