Skip to content

Commit

Permalink
Do leak check after function ptr coercion
Browse files Browse the repository at this point in the history
  • Loading branch information
jackh726 committed May 22, 2022
1 parent 4bb4dc4 commit 683a9c8
Show file tree
Hide file tree
Showing 30 changed files with 271 additions and 256 deletions.
11 changes: 8 additions & 3 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1442,6 +1442,10 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
/// the message in `secondary_span` as the primary label, and apply the message that would
/// otherwise be used for the primary label on the `secondary_span` `Span`. This applies on
/// E0271, like `src/test/ui/issues/issue-39970.stderr`.
#[tracing::instrument(
level = "debug",
skip(self, diag, secondary_span, swap_secondary_and_primary, force_label)
)]
pub fn note_type_err(
&self,
diag: &mut Diagnostic,
Expand All @@ -1453,7 +1457,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
force_label: bool,
) {
let span = cause.span(self.tcx);
debug!("note_type_err cause={:?} values={:?}, terr={:?}", cause, values, terr);

// For some types of errors, expected-found does not make
// sense, so just ignore the values we were given.
Expand Down Expand Up @@ -1621,9 +1624,9 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
}
};

// Ignore msg for object safe coercion
// since E0038 message will be printed
match terr {
// Ignore msg for object safe coercion
// since E0038 message will be printed
TypeError::ObjectUnsafeCoercion(_) => {}
_ => {
let mut label_or_note = |span: Span, msg: &str| {
Expand Down Expand Up @@ -1774,6 +1777,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// It reads better to have the error origin as the final
// thing.
self.note_error_origin(diag, cause, exp_found, terr);

debug!(?diag);
}

fn suggest_tuple_pattern(
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_middle/src/ty/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,10 @@ impl<'tcx> fmt::Display for TypeError<'tcx> {
ArgCount => write!(f, "incorrect number of function parameters"),
FieldMisMatch(adt, field) => write!(f, "field type mismatch: {}.{}", adt, field),
RegionsDoesNotOutlive(..) => write!(f, "lifetime mismatch"),
RegionsInsufficientlyPolymorphic(br, _) => write!(
f,
"expected bound lifetime parameter{}, found concrete lifetime",
br_string(br)
),
// Actually naming the region here is a bit confusing because context is lacking
RegionsInsufficientlyPolymorphic(..) => {
write!(f, "one type is more general than the other")
}
RegionsOverlyPolymorphic(br, _) => write!(
f,
"expected concrete lifetime, found bound lifetime parameter{}",
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
///
/// * From each pre-binding block to the next pre-binding block.
/// * From each otherwise block to the next pre-binding block.
#[tracing::instrument(level = "debug", skip(self, arms))]
pub(crate) fn match_expr(
&mut self,
destination: Place<'tcx>,
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/thir/cx/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ impl<'tcx> Cx<'tcx> {
};

let mut pattern = self.pattern_from_hir(local.pat);
debug!(?pattern);

if let Some(ty) = &local.ty {
if let Some(&user_ty) =
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_build/src/thir/cx/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ impl<'tcx> Cx<'tcx> {
}
}

#[tracing::instrument(level = "debug", skip(self))]
pub(crate) fn pattern_from_hir(&mut self, p: &hir::Pat<'_>) -> Pat<'tcx> {
let p = match self.tcx.hir().get(p.hir_id) {
Node::Pat(p) | Node::Binding(p) => p,
Expand Down
73 changes: 41 additions & 32 deletions compiler/rustc_typeck/src/check/_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut all_arms_diverge = Diverges::WarnedAlways;

let expected = orig_expected.adjust_for_branches(self);
debug!(?expected);

let mut coercion = {
let coerce_first = match expected {
Expand Down Expand Up @@ -127,6 +128,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Some(&arm.body),
arm_ty,
Some(&mut |err: &mut Diagnostic| {
let Some(ret) = self.ret_type_span else {
return;
};
let Expectation::IsLast(stmt) = orig_expected else {
return
};
let can_coerce_to_return_ty = match self.ret_coercion.as_ref() {
Some(ret_coercion) if self.in_tail_expr => {
let ret_ty = ret_coercion.borrow().expected_ty();
Expand All @@ -138,38 +145,38 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
_ => false,
};
if let (Expectation::IsLast(stmt), Some(ret), true) =
(orig_expected, self.ret_type_span, can_coerce_to_return_ty)
{
let semi_span = expr.span.shrink_to_hi().with_hi(stmt.hi());
let mut ret_span: MultiSpan = semi_span.into();
ret_span.push_span_label(
expr.span,
"this could be implicitly returned but it is a statement, not a \
tail expression"
.to_owned(),
);
ret_span.push_span_label(
ret,
"the `match` arms can conform to this return type".to_owned(),
);
ret_span.push_span_label(
semi_span,
"the `match` is a statement because of this semicolon, consider \
removing it"
.to_owned(),
);
err.span_note(
ret_span,
"you might have meant to return the `match` expression",
);
err.tool_only_span_suggestion(
semi_span,
"remove this semicolon",
String::new(),
Applicability::MaybeIncorrect,
);
if !can_coerce_to_return_ty {
return;
}

let semi_span = expr.span.shrink_to_hi().with_hi(stmt.hi());
let mut ret_span: MultiSpan = semi_span.into();
ret_span.push_span_label(
expr.span,
"this could be implicitly returned but it is a statement, not a \
tail expression"
.to_owned(),
);
ret_span.push_span_label(
ret,
"the `match` arms can conform to this return type".to_owned(),
);
ret_span.push_span_label(
semi_span,
"the `match` is a statement because of this semicolon, consider \
removing it"
.to_owned(),
);
err.span_note(
ret_span,
"you might have meant to return the `match` expression",
);
err.tool_only_span_suggestion(
semi_span,
"remove this semicolon",
String::new(),
Applicability::MaybeIncorrect,
);
}),
false,
);
Expand Down Expand Up @@ -199,7 +206,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// We won't diverge unless the scrutinee or all arms diverge.
self.diverges.set(scrut_diverges | all_arms_diverge);

coercion.complete(self)
let match_ty = coercion.complete(self);
debug!(?match_ty);
match_ty
}

fn get_appropriate_arm_semicolon_removal_span(
Expand Down
47 changes: 32 additions & 15 deletions compiler/rustc_typeck/src/check/coercion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -737,14 +737,27 @@ impl<'f, 'tcx> Coerce<'f, 'tcx> {
F: FnOnce(Ty<'tcx>) -> Vec<Adjustment<'tcx>>,
G: FnOnce(Ty<'tcx>) -> Vec<Adjustment<'tcx>>,
{
if let ty::FnPtr(fn_ty_b) = b.kind()
&& let (hir::Unsafety::Normal, hir::Unsafety::Unsafe) =
(fn_ty_a.unsafety(), fn_ty_b.unsafety())
{
let unsafe_a = self.tcx.safe_to_unsafe_fn_ty(fn_ty_a);
return self.unify_and(unsafe_a, b, to_unsafe);
}
self.unify_and(a, b, normal)
self.commit_unconditionally(|snapshot| {
let result = if let ty::FnPtr(fn_ty_b) = b.kind()
&& let (hir::Unsafety::Normal, hir::Unsafety::Unsafe) =
(fn_ty_a.unsafety(), fn_ty_b.unsafety())
{
let unsafe_a = self.tcx.safe_to_unsafe_fn_ty(fn_ty_a);
self.unify_and(unsafe_a, b, to_unsafe)
} else {
self.unify_and(a, b, normal)
};

// FIXME(#73154): This is a hack. Currently LUB can generate
// unsolvable constraints. Additionally, it returns `a`
// unconditionally, even when the "LUB" is `b`. In the future, we
// want the coerced type to be the actual supertype of these two,
// but for now, we want to just error to ensure we don't lock
// ourselves into a specific behavior with NLL.
self.leak_check(false, snapshot)?;

result
})
}

fn coerce_from_fn_pointer(
Expand Down Expand Up @@ -1133,8 +1146,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let (adjustments, target) = self.register_infer_ok_obligations(ok);
self.apply_adjustments(new, adjustments);
debug!(
"coercion::try_find_coercion_lub: was able to coerce from previous type {:?} to new type {:?}",
prev_ty, new_ty,
"coercion::try_find_coercion_lub: was able to coerce from new type {:?} to previous type {:?} ({:?})",
new_ty, prev_ty, target
);
return Ok(target);
}
Expand Down Expand Up @@ -1190,15 +1203,15 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
Ok(ok) => {
debug!(
"coercion::try_find_coercion_lub: was able to coerce previous type {:?} to new type {:?}",
prev_ty, new_ty,
);
let (adjustments, target) = self.register_infer_ok_obligations(ok);
for expr in exprs {
let expr = expr.as_coercion_site();
self.apply_adjustments(expr, adjustments.clone());
}
debug!(
"coercion::try_find_coercion_lub: was able to coerce previous type {:?} to new type {:?} ({:?})",
prev_ty, new_ty, target
);
Ok(target)
}
}
Expand Down Expand Up @@ -1430,6 +1443,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
})
};

debug!(?result);
match result {
Ok(v) => {
self.final_ty = Some(v);
Expand Down Expand Up @@ -1520,7 +1534,10 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
augment_error(&mut err);
}

if let Some(expr) = expression {
let is_insufficiently_polymorphic =
matches!(coercion_error, TypeError::RegionsInsufficientlyPolymorphic(..));

if !is_insufficiently_polymorphic && let Some(expr) = expression {
fcx.emit_coerce_suggestions(
&mut err,
expr,
Expand Down
18 changes: 17 additions & 1 deletion compiler/rustc_typeck/src/check/demand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
///
/// N.B., this code relies on `self.diverges` to be accurate. In particular, assignments to `!`
/// will be permitted if the diverges flag is currently "always".
#[tracing::instrument(level = "debug", skip(self, expr, expected_ty_expr, allow_two_phase))]
pub fn demand_coerce_diag(
&self,
expr: &hir::Expr<'tcx>,
Expand All @@ -150,7 +151,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let expr_ty = self.resolve_vars_with_obligations(checked_ty);
let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e.clone());

self.emit_coerce_suggestions(&mut err, expr, expr_ty, expected, expected_ty_expr, Some(e));
let is_insufficiently_polymorphic =
matches!(e, TypeError::RegionsInsufficientlyPolymorphic(..));

// FIXME(#73154): For now, we do leak check when coercing function
// pointers in typeck, instead of only during borrowck. This can lead
// to these `RegionsInsufficientlyPolymorphic` errors that aren't helpful.
if !is_insufficiently_polymorphic {
self.emit_coerce_suggestions(
&mut err,
expr,
expr_ty,
expected,
expected_ty_expr,
Some(e),
);
}

(expected, Some(err))
}
Expand Down
31 changes: 3 additions & 28 deletions src/test/ui/hr-subtype/placeholder-pattern-fail.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,37 +1,12 @@
error[E0308]: mismatched types
--> $DIR/placeholder-pattern-fail.rs:9:12
--> $DIR/placeholder-pattern-fail.rs:9:47
|
LL | let _: for<'a, 'b> fn(Inv<'a>, Inv<'b>) = sub;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
| ^^^ one type is more general than the other
|
= note: expected fn pointer `for<'a, 'b> fn(Inv<'a>, Inv<'b>)`
found fn pointer `for<'a> fn(Inv<'a>, Inv<'a>)`

error[E0308]: mismatched types
--> $DIR/placeholder-pattern-fail.rs:9:12
|
LL | let _: for<'a, 'b> fn(Inv<'a>, Inv<'b>) = sub;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ one type is more general than the other
|
= note: expected fn pointer `for<'a, 'b> fn(Inv<'a>, Inv<'b>)`
found fn pointer `for<'a> fn(Inv<'a>, Inv<'a>)`

error: lifetime may not live long enough
--> $DIR/placeholder-pattern-fail.rs:14:13
|
LL | fn simple1<'c>(x: (&'c i32,)) {
| -- lifetime `'c` defined here
LL | let _x: (&'static i32,) = x;
| ^^^^^^^^^^^^^^^ type annotation requires that `'c` must outlive `'static`

error: lifetime may not live long enough
--> $DIR/placeholder-pattern-fail.rs:19:12
|
LL | fn simple2<'c>(x: (&'c i32,)) {
| -- lifetime `'c` defined here
LL | let _: (&'static i32,) = x;
| ^^^^^^^^^^^^^^^ type annotation requires that `'c` must outlive `'static`

error: aborting due to 4 previous errors
error: aborting due to previous error

For more information about this error, try `rustc --explain E0308`.
4 changes: 2 additions & 2 deletions src/test/ui/hrtb/hrtb-exists-forall-fn.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error[E0308]: mismatched types
--> $DIR/hrtb-exists-forall-fn.rs:17:12
--> $DIR/hrtb-exists-forall-fn.rs:17:34
|
LL | let _: for<'b> fn(&'b u32) = foo();
| ^^^^^^^^^^^^^^^^^^^ one type is more general than the other
| ^^^^^ one type is more general than the other
|
= note: expected fn pointer `for<'b> fn(&'b u32)`
found fn pointer `fn(&u32)`
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
error[E0308]: `match` arms have incompatible types
--> $DIR/old-lub-glb-hr-noteq1.rs:11:14
--> $DIR/old-lub-glb-hr-noteq1.rs:17:14
|
LL | let z = match 22 {
| _____________-
LL | | 0 => x,
| | - this is found to be of type `for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8`
LL | | _ => y,
| | ^ one type is more general than the other
LL | |
... |
LL | |
LL | | };
| |_____- `match` arms have incompatible types
|
Expand Down
21 changes: 21 additions & 0 deletions src/test/ui/lub-glb/old-lub-glb-hr-noteq1.basenoleak.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
error[E0308]: `match` arms have incompatible types
--> $DIR/old-lub-glb-hr-noteq1.rs:17:14
|
LL | let z = match 22 {
| _____________-
LL | | 0 => x,
| | - this is found to be of type `for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8`
LL | | _ => y,
| | ^ one type is more general than the other
LL | |
... |
LL | |
LL | | };
| |_____- `match` arms have incompatible types
|
= note: expected fn pointer `for<'a, 'b> fn(&'a u8, &'b u8) -> &'a u8`
found fn pointer `for<'a> fn(&'a u8, &'a u8) -> &'a u8`

error: aborting due to previous error

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

0 comments on commit 683a9c8

Please sign in to comment.