Skip to content

Try to use approximate placeholder regions when outputting an AscribeUserType error in borrowck #116097

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

Merged
merged 1 commit into from
Nov 16, 2023
Merged
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
50 changes: 38 additions & 12 deletions compiler/rustc_borrowck/src/diagnostics/bound_region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ use rustc_infer::infer::RegionVariableOrigin;
use rustc_infer::infer::{InferCtxt, RegionResolutionError, SubregionOrigin, TyCtxtInferExt as _};
use rustc_infer::traits::ObligationCause;
use rustc_middle::ty::error::TypeError;
use rustc_middle::ty::RePlaceholder;
use rustc_middle::ty::Region;
use rustc_middle::ty::RegionVid;
use rustc_middle::ty::UniverseIndex;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};
Expand Down Expand Up @@ -205,6 +207,8 @@ trait TypeOpInfo<'tcx> {
let span = cause.span;
let nice_error = self.nice_error(mbcx, cause, placeholder_region, error_region);

debug!(?nice_error);

if let Some(nice_error) = nice_error {
mbcx.buffer_error(nice_error);
} else {
Expand Down Expand Up @@ -404,19 +408,41 @@ fn try_extract_error_from_region_constraints<'tcx>(
mut region_var_origin: impl FnMut(RegionVid) -> RegionVariableOrigin,
mut universe_of_region: impl FnMut(RegionVid) -> UniverseIndex,
) -> Option<DiagnosticBuilder<'tcx, ErrorGuaranteed>> {
let (sub_region, cause) =
region_constraints.constraints.iter().find_map(|(constraint, cause)| {
match *constraint {
Constraint::RegSubReg(sub, sup) if sup == placeholder_region && sup != sub => {
Some((sub, cause.clone()))
}
// FIXME: Should this check the universe of the var?
Constraint::VarSubReg(vid, sup) if sup == placeholder_region => {
Some((ty::Region::new_var(infcx.tcx, vid), cause.clone()))
}
_ => None,
let matches =
|a_region: Region<'tcx>, b_region: Region<'tcx>| match (a_region.kind(), b_region.kind()) {
(RePlaceholder(a_p), RePlaceholder(b_p)) => a_p.bound == b_p.bound,
_ => a_region == b_region,
};
let check = |constraint: &Constraint<'tcx>, cause: &SubregionOrigin<'tcx>, exact| {
match *constraint {
Constraint::RegSubReg(sub, sup)
if ((exact && sup == placeholder_region)
|| (!exact && matches(sup, placeholder_region)))
&& sup != sub =>
{
Some((sub, cause.clone()))
}
// FIXME: Should this check the universe of the var?
Constraint::VarSubReg(vid, sup)
if ((exact && sup == placeholder_region)
|| (!exact && matches(sup, placeholder_region))) =>
{
Some((ty::Region::new_var(infcx.tcx, vid), cause.clone()))
}
})?;
_ => None,
}
};
let mut info = region_constraints
.constraints
.iter()
.find_map(|(constraint, cause)| check(constraint, cause, true));
if info.is_none() {
info = region_constraints
.constraints
.iter()
.find_map(|(constraint, cause)| check(constraint, cause, false));
}
let (sub_region, cause) = info?;

debug!(?sub_region, "cause = {:#?}", cause);
let error = match (error_region, *sub_region) {
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_borrowck/src/diagnostics/region_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,12 @@ impl<'tcx> RegionErrors<'tcx> {
}
}

impl std::fmt::Debug for RegionErrors<'_> {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_tuple("RegionErrors").field(&self.0).finish()
}
}

#[derive(Clone, Debug)]
pub(crate) enum RegionErrorKind<'tcx> {
/// A generic bound failure for a type test (`T: 'a`).
Expand Down
9 changes: 9 additions & 0 deletions compiler/rustc_borrowck/src/region_infer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ pub(crate) struct AppliedMemberConstraint {
pub(crate) member_constraint_index: NllMemberConstraintIndex,
}

#[derive(Debug)]
pub(crate) struct RegionDefinition<'tcx> {
/// What kind of variable is this -- a free region? existential
/// variable? etc. (See the `NllRegionVariableOrigin` for more
Expand Down Expand Up @@ -680,6 +681,9 @@ impl<'tcx> RegionInferenceContext<'tcx> {
&mut errors_buffer,
);

debug!(?errors_buffer);
debug!(?outlives_requirements);

// In Polonius mode, the errors about missing universal region relations are in the output
// and need to be emitted or propagated. Otherwise, we need to check whether the
// constraints were too strong, and if so, emit or propagate those errors.
Expand All @@ -693,10 +697,14 @@ impl<'tcx> RegionInferenceContext<'tcx> {
self.check_universal_regions(outlives_requirements.as_mut(), &mut errors_buffer);
}

debug!(?errors_buffer);

if errors_buffer.is_empty() {
self.check_member_constraints(infcx, &mut errors_buffer);
}

debug!(?errors_buffer);

let outlives_requirements = outlives_requirements.unwrap_or_default();

if outlives_requirements.is_empty() {
Expand Down Expand Up @@ -1450,6 +1458,7 @@ impl<'tcx> RegionInferenceContext<'tcx> {
errors_buffer: &mut RegionErrors<'tcx>,
) {
for (fr, fr_definition) in self.definitions.iter_enumerated() {
debug!(?fr, ?fr_definition);
match fr_definition.origin {
NllRegionVariableOrigin::FreeRegion => {
// Go through each of the universal regions `fr` and check that
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1181,7 +1181,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
self.infcx.tcx
}

#[instrument(skip(self, body, location), level = "debug")]
#[instrument(skip(self, body), level = "debug")]
fn check_stmt(&mut self, body: &Body<'tcx>, stmt: &Statement<'tcx>, location: Location) {
let tcx = self.tcx();
debug!("stmt kind: {:?}", stmt.kind);
Expand Down
14 changes: 14 additions & 0 deletions tests/ui/higher-ranked/higher-ranked-lifetime-error.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
fn assert_all<F, T>(_f: F)
where
F: FnMut(&String) -> T,
{
}

fn id(x: &String) -> &String {
x
}

fn main() {
assert_all::<_, &String>(id);
//~^ mismatched types
}
12 changes: 12 additions & 0 deletions tests/ui/higher-ranked/higher-ranked-lifetime-error.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
error[E0308]: mismatched types
--> $DIR/higher-ranked-lifetime-error.rs:12:5
|
LL | assert_all::<_, &String>(id);
| ^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected reference `&String`
found reference `&String`
Comment on lines +7 to +8
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if I consider this an improvement over "higher ranked lifetime error".

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the note is not all that helpful. But its actually very similar to our pre-NLL error (see output on 1.64).

I consider this more of a "get us back to where we were" than a "this is the best we can do"

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 "one type is more general than the other" note is the most helpful part of the diagnostic, and I think that's an improvement over "higher-ranked lifetime error" alone.


error: aborting due to previous error

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