diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2024-08-17 18:18:19 +0200 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-08-17 18:18:19 +0200 |
| commit | ddbbda47ebf2a69008f48b9c5e83b615e0e00545 (patch) | |
| tree | 0b921132e0ed2da427debf73e30a3c3a7d870015 /compiler | |
| parent | 9c910ae7ee6207d6b190e5543eecdba5b4c15273 (diff) | |
| parent | ed6315b3fef4ebd9aee053d407eb746c3b1d58bf (diff) | |
| download | rust-ddbbda47ebf2a69008f48b9c5e83b615e0e00545.tar.gz rust-ddbbda47ebf2a69008f48b9c5e83b615e0e00545.zip | |
Rollup merge of #129168 - BoxyUwU:mismatched_ty_correct_id, r=compiler-errors
Return correct HirId when finding body owner in diagnostics
Fixes #129145
Fixes #128810
r? ```@compiler-errors```
```rust
fn generic<const N: u32>() {}
trait Collate<const A: u32> {
type Pass;
fn collate(self) -> Self::Pass;
}
impl<const B: u32> Collate<B> for i32 {
type Pass = ();
fn collate(self) -> Self::Pass {
generic::<{ true }>()
//~^ ERROR: mismatched types
}
}
```
When type checking the `{ true }` anon const we would error with a type mismatch. This then results in diagnostics code attempting to check whether its due to a type mismatch with the return type. That logic was implemented by walking up the hir until we reached the body owner, except instead of using the `enclosing_body_owner` function it special cased various hir nodes incorrectly resulting in us walking out of the anon const and stopping at `fn collate` instead.
This then resulted in diagnostics logic inside of the anon consts `ParamEnv` attempting to do trait solving involving the `<i32 as Collate<B>>::Pass` type which ICEs because it is in the wrong environment.
I have rewritten this function to just walk up until it hits the `enclosing_body_owner` and made some other changes since I found this pretty hard to read/understand. Hopefully it's easier to understand now, it also makes it more obvious that this is not implemented in a very principled way and is definitely missing cases :)
Diffstat (limited to 'compiler')
| -rw-r--r-- | compiler/rustc_middle/src/hir/map/mod.rs | 64 |
1 files changed, 27 insertions, 37 deletions
diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index edab6b5ebde..da08027d66b 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -554,53 +554,43 @@ impl<'hir> Map<'hir> { /// } /// ``` pub fn get_fn_id_for_return_block(self, id: HirId) -> Option<HirId> { - let mut iter = self.parent_iter(id).peekable(); - let mut ignore_tail = false; - if let Node::Expr(Expr { kind: ExprKind::Ret(_), .. }) = self.tcx.hir_node(id) { - // When dealing with `return` statements, we don't care about climbing only tail - // expressions. - ignore_tail = true; - } + let enclosing_body_owner = self.tcx.local_def_id_to_hir_id(self.enclosing_body_owner(id)); + + // Return `None` if the `id` expression is not the returned value of the enclosing body + let mut iter = [id].into_iter().chain(self.parent_id_iter(id)).peekable(); + while let Some(cur_id) = iter.next() { + if enclosing_body_owner == cur_id { + break; + } + + // A return statement is always the value returned from the enclosing body regardless of + // what the parent expressions are. + if let Node::Expr(Expr { kind: ExprKind::Ret(_), .. }) = self.tcx.hir_node(cur_id) { + break; + } - let mut prev_hir_id = None; - while let Some((hir_id, node)) = iter.next() { - if let (Some((_, next_node)), false) = (iter.peek(), ignore_tail) { - match next_node { - Node::Block(Block { expr: None, .. }) => return None, - // The current node is not the tail expression of its parent. - Node::Block(Block { expr: Some(e), .. }) if hir_id != e.hir_id => return None, + // If the current expression's value doesnt get used as the parent expressions value then return `None` + if let Some(&parent_id) = iter.peek() { + match self.tcx.hir_node(parent_id) { + // The current node is not the tail expression of the block expression parent expr. + Node::Block(Block { expr: Some(e), .. }) if cur_id != e.hir_id => return None, Node::Block(Block { expr: Some(e), .. }) if matches!(e.kind, ExprKind::If(_, _, None)) => { return None; } + + // The current expression's value does not pass up through these parent expressions + Node::Block(Block { expr: None, .. }) + | Node::Expr(Expr { kind: ExprKind::Loop(..), .. }) + | Node::LetStmt(..) => return None, + _ => {} } } - match node { - Node::Item(_) - | Node::ForeignItem(_) - | Node::TraitItem(_) - | Node::Expr(Expr { kind: ExprKind::Closure(_), .. }) - | Node::ImplItem(_) - // The input node `id` must be enclosed in the method's body as opposed - // to some other place such as its return type (fixes #114918). - // We verify that indirectly by checking that the previous node is the - // current node's body - if node.body_id().map(|b| b.hir_id) == prev_hir_id => { - return Some(hir_id) - } - // Ignore `return`s on the first iteration - Node::Expr(Expr { kind: ExprKind::Loop(..) | ExprKind::Ret(..), .. }) - | Node::LetStmt(_) => { - return None; - } - _ => {} - } - - prev_hir_id = Some(hir_id); } - None + + Some(enclosing_body_owner) } /// Retrieves the `OwnerId` for `id`'s parent item, or `id` itself if no |
