save-analysis: fix a bug with tuple sub-expressions #34327
Conversation
|
r? @arielb1 (rust_highfive has picked a reviewer for you, use r? to override) |
|
I fixed the underlying cause of this bug in #34355. |
|
ping @arielb1 for review re @jseyfried 's comment, even if the underlying cause of the bug is fixed, I would like this to land since it adds a layer of safety. |
| Some(Node::NodeExpr(expr)) => expr, | ||
| _ => { | ||
| debug!("Missing or weird node for sub-expression {} in {:?}", | ||
| sub_ex.id, ex); |
There was a problem hiding this comment.
nit: If sub_ex.id is not mapped to an hir expression node, I think that's solely a problem with sub_ex and sub_ex.id -- I don't think ex is relevant.
There was a problem hiding this comment.
My reasoning is that if subex is messed up for some other reason, then it is useful to have an expression which we know is not messed up. I also find it easier to debug this way - you get a reference to say (a, b, x + 4) rather than a and the former is easier to grep for.
|
@nrc r=me with or without the nit |
…nique, r=nrc Give `ast::ExprKind::Paren` no-op expressions the same ids as their children. Having `ast::ExprKind::Paren` expressions share ids with their children - reduces the number of unused `NodeId`s in the hir map and - guarantees that `tcx.map.expect_expr(ast_expr.id)` is the hir corresponding to `ast_expr`. This fixes the bug from rust-lang#34327, which was introduced in rust-lang#33296 when I assumed the above guarantee. r? @nrc
|
@bors: r=jseyfried rollup |
|
📌 Commit 60e19ce has been approved by |
save-analysis: fix a bug with tuple sub-expressions
No description provided.