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

fallback to resolve infer generics in type-changing-struct-update #102129

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions compiler/rustc_hir_typeck/src/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed
use rustc_hir as hir;
use rustc_hir::lang_items::LangItem;
use rustc_hir::{is_range_literal, Node};
use rustc_infer::infer::base_struct::base_struct;
use rustc_infer::infer::InferOk;
use rustc_middle::lint::in_external_macro;
use rustc_middle::middle::stability::EvalResult;
Expand Down Expand Up @@ -174,6 +175,23 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
(expected, Some(err))
}

#[instrument(skip(self), level = "debug")]
pub fn demand_base_struct(
&self,
cause: &ObligationCause<'tcx>,
adt_ty: Ty<'tcx>,
base_ty: Ty<'tcx>,
) -> Option<DiagnosticBuilder<'tcx, ErrorGuaranteed>> {
let at = self.at(cause, self.param_env);
match base_struct(at, adt_ty, base_ty) {
Ok(InferOk { obligations, value: () }) => {
self.register_predicates(obligations);
None
}
Err(e) => Some(self.err_ctxt().report_mismatched_types(&cause, adt_ty, base_ty, e)),
}
}

fn annotate_expected_due_to_let_ty(
&self,
err: &mut Diagnostic,
Expand Down
28 changes: 11 additions & 17 deletions compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ use rustc_infer::infer::InferOk;
use rustc_infer::traits::ObligationCause;
use rustc_middle::middle::stability;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AllowTwoPhase};
use rustc_middle::ty::error::TypeError::FieldMisMatch;
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, AdtKind, Ty, TypeVisitable};
use rustc_session::errors::ExprParenthesesNeeded;
Expand Down Expand Up @@ -1643,7 +1642,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
if let Some(base_expr) = base_expr {
// FIXME: We are currently creating two branches here in order to maintain
// consistency. But they should be merged as much as possible.
let fru_tys = if self.tcx.features().type_changing_struct_update {
let fru_tys = if self.tcx.features().type_changing_struct_update || true {
if adt.is_struct() {
// Make some fresh substitutions for our ADT type.
let fresh_substs = self.fresh_substs_for_item(base_expr.span, adt.did());
Expand All @@ -1668,16 +1667,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
self.register_predicates(obligations)
}
Err(_) => {
// This should never happen, since we're just subtyping the
// remaining_fields, but it's fine to emit this, I guess.
self.err_ctxt()
.report_mismatched_types(
&cause,
target_ty,
fru_ty,
FieldMisMatch(variant.name, ident.name),
)
.emit();
bug!("subtype fresh substs failed")
}
}
}
Expand All @@ -1703,11 +1693,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// This is important to allow coercions to happen in
// `other_struct` itself. See `coerce-in-base-expr.rs`.
let fresh_base_ty = self.tcx.mk_adt(*adt, fresh_substs);
self.check_expr_has_type_or_error(
base_expr,
self.resolve_vars_if_possible(fresh_base_ty),
|_| {},
);
self.check_expr_has_type_or_error(base_expr, fresh_base_ty, |_| {});
let fresh_base_ty = self.resolve_vars_if_possible(fresh_base_ty);
if fresh_base_ty.needs_infer() {
self.typeck_results.borrow_mut().base_expr_backup.push((
base_expr.span,
adt_ty,
fresh_base_ty,
));
}
fru_tys
} else {
// Check the base_expr, regardless of a bad expected adt_ty, so we can get
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_hir_typeck/src/fn_ctxt/_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}

pub(in super::super) fn resolve_base_expr(&self) {
for (span, adt_ty, base_ty) in self.typeck_results.borrow().base_expr_backup.iter() {
let base_ty = self.resolve_vars_if_possible(*base_ty);
if base_ty.needs_infer() {
if let Some(mut err) = self.demand_base_struct(&self.misc(*span), *adt_ty, base_ty)
{
err.emit();
}
}
}
}

#[instrument(skip(self), level = "debug")]
pub(in super::super) fn select_all_obligations_or_error(&self) {
let mut errors = self.fulfillment_cx.borrow_mut().select_all_or_error(&self);
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,10 @@ fn typeck_with_fallback<'tcx>(
fcx.require_type_is_sized(ty, span, code);
}

// If the base structs used to creat structs still remain infer generics,
// unify it with the created ones.
fcx.resolve_base_expr();

fcx.select_all_obligations_or_error();

if !fcx.infcx.is_tainted_by_errors() {
Expand Down
48 changes: 48 additions & 0 deletions compiler/rustc_infer/src/infer/base_struct.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
use crate::infer::at::{At, ToTrace};
use crate::infer::sub::Sub;
use crate::infer::{InferOk, InferResult};
use rustc_middle::ty;
use rustc_middle::ty::relate::{relate_generic_arg, RelateResult, TypeRelation};
use rustc_middle::ty::{GenericArg, SubstsRef, Ty};
use std::iter;

pub fn base_struct<'a, 'tcx>(at: At<'a, 'tcx>, a: Ty<'tcx>, b: Ty<'tcx>) -> InferResult<'tcx, ()> {
let trace = ToTrace::to_trace(at.infcx.tcx, at.cause, true, a, b);
at.infcx.commit_if_ok(|_| {
let mut fields = at.infcx.combine_fields(trace, at.param_env, at.define_opaque_types);
let mut sub = Sub::new(&mut fields, true);
base_struct_tys(&mut sub, a, b)
.map(move |_| InferOk { value: (), obligations: fields.obligations })
})
}

pub fn base_struct_tys<'tcx>(
sub: &mut Sub<'_, '_, 'tcx>,
a: Ty<'tcx>,
b: Ty<'tcx>,
) -> RelateResult<'tcx, ()> {
match (a.kind(), b.kind()) {
(&ty::Adt(a_def, a_substs), &ty::Adt(b_def, b_substs)) if a_def == b_def => {
base_struct_substs(sub, a, a_substs, b_substs)?;
Ok(())
}
_ => bug!("not adt ty: {:?} and {:?}", a, b),
}
}

fn base_struct_substs<'tcx>(
sub: &mut Sub<'_, '_, 'tcx>,
adt_ty: Ty<'tcx>,
a_subst: SubstsRef<'tcx>,
b_subst: SubstsRef<'tcx>,
) -> RelateResult<'tcx, ()> {
let tcx = sub.tcx();
let variances = tcx.variances_of(adt_ty.ty_adt_def().expect("not a adt ty!").did());

iter::zip(a_subst, b_subst).enumerate().for_each(|(i, (a, b))| {
let _arg: RelateResult<'tcx, GenericArg<'tcx>> =
Copy link
Member

Choose a reason for hiding this comment

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

Why are you throwing this result away?

Copy link
Member Author

Choose a reason for hiding this comment

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

The purpose of this code is to instantiate the uninstantiated infer generic created for base_expr in check_expr_struct_fields by calling the relate_generic_arg function. If Err is returned, the generic type has been instantiated by the type of base_expr itself and is not constrained by the created struct (constrained ones hav been checked in check_expr_struct_fields).

Copy link
Member Author

Choose a reason for hiding this comment

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

Perhaps we can have a more accurate approach, such as maintaining a non-constrained generic indexes list for base_expr.

Copy link
Contributor

Choose a reason for hiding this comment

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

Given that this is purely for diagnostics, this approach might be fine, but including what you wrote above as a comment would be welcome.

Copy link
Member

Choose a reason for hiding this comment

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

Given that this is purely for diagnostics

I'm confused by this comment. This isn't purely diagnostic, I thought? My understanding is that this function affects type inference -- notably, relate_generic_arg is also not an atomic operation, so even if it returns Err, it can still partially constrain inference variables inside of that.

I'm not convinced that this approach is completely sound. I would be comfortable with something more along the lines of:

such as maintaining a non-constrained generic indexes list for base_expr.

At least in that case it's easier to reason about when we're allowed to unify unconstrained inference variables and when we expect to raise errors.

relate_generic_arg(sub, variances, adt_ty, a, b, i).or_else(|_| Ok(b));
});

Ok(())
}
1 change: 1 addition & 0 deletions compiler/rustc_infer/src/infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ use self::region_constraints::{
use self::type_variable::{TypeVariableOrigin, TypeVariableOriginKind};

pub mod at;
pub mod base_struct;
pub mod canonical;
mod combine;
mod equate;
Expand Down
5 changes: 5 additions & 0 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,10 @@ pub struct TypeckResults<'tcx> {
/// Contains the data for evaluating the effect of feature `capture_disjoint_fields`
/// on closure size.
pub closure_size_eval: FxHashMap<LocalDefId, ClosureSizeProfileData<'tcx>>,

/// Hold the generic parameters that were not fully instantiated when
/// creating a struct from a base struct.
pub base_expr_backup: Vec<(Span, Ty<'tcx>, Ty<'tcx>)>,
}

impl<'tcx> TypeckResults<'tcx> {
Expand All @@ -599,6 +603,7 @@ impl<'tcx> TypeckResults<'tcx> {
closure_kind_origins: Default::default(),
liberated_fn_sigs: Default::default(),
fru_field_types: Default::default(),
base_expr_backup: Default::default(),
coercion_casts: Default::default(),
used_trait_imports: Lrc::new(Default::default()),
tainted_by_errors: None,
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ pub enum TypeError<'tcx> {
TupleSize(ExpectedFound<usize>),
FixedArraySize(ExpectedFound<u64>),
ArgCount,
FieldMisMatch(Symbol, Symbol),

RegionsDoesNotOutlive(Region<'tcx>, Region<'tcx>),
RegionsInsufficientlyPolymorphic(BoundRegionKind, Region<'tcx>),
Expand Down Expand Up @@ -147,7 +146,6 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
pluralize!(values.found)
),
ArgCount => write!(f, "incorrect number of function parameters"),
FieldMisMatch(adt, field) => write!(f, "field type mismatch: {}.{}", adt, field),
RegionsDoesNotOutlive(..) => write!(f, "lifetime mismatch"),
// Actually naming the region here is a bit confusing because context is lacking
RegionsInsufficientlyPolymorphic(..) => {
Expand Down Expand Up @@ -237,7 +235,6 @@ impl<'tcx> TypeError<'tcx> {
| ArgumentMutability(_)
| TupleSize(_)
| ArgCount
| FieldMisMatch(..)
| RegionsDoesNotOutlive(..)
| RegionsInsufficientlyPolymorphic(..)
| RegionsOverlyPolymorphic(..)
Expand Down
30 changes: 21 additions & 9 deletions compiler/rustc_middle/src/ty/relate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,20 +156,32 @@ pub fn relate_substs_with_variances<'tcx, R: TypeRelation<'tcx>>(

let mut cached_ty = None;
let params = iter::zip(a_subst, b_subst).enumerate().map(|(i, (a, b))| {
let variance = variances[i];
let variance_info = if variance == ty::Invariant {
let ty =
*cached_ty.get_or_insert_with(|| tcx.bound_type_of(ty_def_id).subst(tcx, a_subst));
ty::VarianceDiagInfo::Invariant { ty, param_index: i.try_into().unwrap() }
} else {
ty::VarianceDiagInfo::default()
};
relation.relate_with_variance(variance, variance_info, a, b)
let cached_ty =
*cached_ty.get_or_insert_with(|| tcx.bound_type_of(ty_def_id).subst(tcx, a_subst));
relate_generic_arg(relation, variances, cached_ty, a, b, i)
});

tcx.mk_substs(params)
}

#[inline]
pub fn relate_generic_arg<'tcx, R: TypeRelation<'tcx>>(
relation: &mut R,
variances: &[ty::Variance],
ty: Ty<'tcx>,
a: GenericArg<'tcx>,
b: GenericArg<'tcx>,
index: usize,
) -> RelateResult<'tcx, GenericArg<'tcx>> {
let variance = variances[index];
let variance_info = if variance == ty::Invariant {
ty::VarianceDiagInfo::Invariant { ty, param_index: index.try_into().unwrap() }
} else {
ty::VarianceDiagInfo::default()
};
relation.relate_with_variance(variance, variance_info, a, b)
}

impl<'tcx> Relate<'tcx> for ty::FnSig<'tcx> {
fn relate<R: TypeRelation<'tcx>>(
relation: &mut R,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,9 @@ fn update_to_state2() {
common_field2: 2,
};
let m2: Machine<State2> = Machine {
state: State2,
state: State1,
//~^ ERROR mismatched types [E0308]
..m1
//~^ ERROR type changing struct updating is experimental [E0658]
//~| ERROR mismatched types [E0308]
};
assert_eq!(State2, m2.state);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,9 @@
error[E0658]: type changing struct updating is experimental
--> $DIR/feature-gate.rs:22:11
|
LL | ..m1
| ^^
|
= note: see issue #86555 <https://github.com/rust-lang/rust/issues/86555> for more information
= help: add `#![feature(type_changing_struct_update)]` to the crate attributes to enable

error[E0308]: mismatched types
--> $DIR/feature-gate.rs:22:11
|
LL | ..m1
| ^^ expected struct `State2`, found struct `State1`
--> $DIR/feature-gate.rs:21:16
|
= note: expected struct `Machine<State2>`
found struct `Machine<State1>`
LL | state: State1,
| ^^^^^^ expected struct `State2`, found struct `State1`

error: aborting due to 2 previous errors
error: aborting due to previous error

Some errors have detailed explanations: E0308, E0658.
For more information about an error, try `rustc --explain E0308`.
For more information about this error, try `rustc --explain E0308`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
#![feature(type_changing_struct_update)]
#![allow(incomplete_features)]

#[derive(Default)]
pub struct Foo<P, T> {
pub t: T,
pub v: P
}

impl<P: Default, T: Default> Foo<P, T> {
pub fn new() -> Self {
Foo { t: T::default(), v: P::default() }
}

pub fn d(t: T) -> Self {
Foo { t, ..Default::default() }
}

pub fn o(t: T) -> Self {
let d: Foo<_, _> = Foo::new();
Foo { t, ..d }
}

pub fn o2<P2: Default>(t: T, v: P2) -> (Foo<P, T>, Foo<P2, T>) {
let d = Default::default();
let foo1 = Foo { t, ..d };
let foo2 = Foo { v, ..d };
(foo1, foo2)
}

pub fn o3<T2: Default>(t1: T, t2: T2) -> (Foo<P, T>, Foo<P, T2>) {
let d = Default::default();
let foo1 = Foo { t: t1, ..d };
let foo2 = Foo { t: t2, ..d };
//~^ ERROR mismatched types [E0308]
(foo1, foo2)
}
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
error[E0308]: mismatched types
--> $DIR/issue-101970.rs:34:35
|
LL | impl<P: Default, T: Default> Foo<P, T> {
| - found type parameter
...
LL | pub fn o3<T2: Default>(t1: T, t2: T2) -> (Foo<P, T>, Foo<P, T2>) {
| -- expected type parameter
...
LL | let foo2 = Foo { t: t2, ..d };
| ^ expected type parameter `T2`, found type parameter `T`
|
= note: expected type parameter `T2`
found type parameter `T`
= note: a type parameter was expected, but a different one was found; you might be missing a type parameter or trait bound
= note: for more information, visit https://doc.rust-lang.org/book/ch10-02-traits.html#traits-as-parameters

error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.