Avoid 'no type for expr' error in CastExpr - Closes #5900 #11832
Conversation
|
This seems kinda unfortunate to duplicate what things are consts and a little bit of the typechecking paths. Albeit I don't know much about these code paths, it appears that the error is I remember fixing a bug like this awhile ago and it was fixed somewhere along those lines, but this may also be a different case. |
|
I agree that it's unfortunate, and that it probably shouldn't be the final fix. The error is in node_id_to_type, but it's not quite as straightforward as adding a missing map insert during typechecking. The root of the problem is that during typechecking the compiler sometimes wants to call eval_const_expr_partial, and when that sometimes is a cast expr, and the cast expr itself hasn't yet been typechecked, it blows up. So in #5900, for instance, defining the enum before the mod worked b/c the enum gets typechecked first. I describe an alternate fix at #5900 (comment) that does work by adding to the map, but I went w/ this PR's approach because it's more in the direction of one possible 'real' fix, making const exprs more safely evaluable at any time. |
|
I cleaned this up a bit. The new limited type lookup function in astconv no longer duplicates knowledge of what makes a const, it just looks for primitives and eval_const_expr_partial keeps the knowledge of what makes for a legal const. I also refactored ast_ty_to_ty to use that helper function instead of duplicating it. Also added a test for 9942. |
|
The test failures aren't your fault, I'm trying to fix mac-64 and I'll retry this when I think I have a fix. |
I tried a couple of different ways to squash this, and still don't think this is ideal, but I wanted to get it out for feedback. Closes #5900 Closes #9942 There are a few scenarios where the compiler tries to evaluate CastExprs without the corresponding types being available yet in the type context: #10618, #5900, #9942 This PR takes the approach of having eval_const_expr_partial's CastExpr arm fall back to a limited ast_ty_to_ty call that only checks for (a subset of) valid const types, when the direct type lookup fails. It's kind of hacky, so I understand if you don't want to take this as is. I'd need a little mentoring to get this into better shape, as figuring out the proper fix has been a little daunting. I'm also happy if someone else wants to pick this up and run with it. This closes 5900 and 9942, but only moves the goalposts a little on 10618, which now falls over in a later phase of the compiler.
|
@alexcrichton I prefer to believe the emoji goat sacrifice worked :) |
|
You have satisfied bors! For now. |
…p1995 Add `lint_groups_priority` lint Warns when a lint group in Cargo.toml's `[lints]` section shares the same priority as a lint. This is in the cargo section but is categorised as `correctness` so it's on by default, it doesn't call `cargo metadata` though and parses the `Cargo.toml` directly The lint should be temporary until rust-lang/cargo#12918 is resolved, but in the meanwhile this is an common issue to run into - rust-lang#11237 - rust-lang#11751 - rust-lang#11830 changelog: Add [`lint_groups_priority`] lint r? `@flip1995`
I tried a couple of different ways to squash this, and still don't think this is ideal, but I wanted to get it out for feedback.
Closes #5900
Closes #9942
There are a few scenarios where the compiler tries to evaluate CastExprs without the corresponding types being available yet in the type context: #10618, #5900, #9942
This PR takes the approach of having eval_const_expr_partial's CastExpr arm fall back to a limited ast_ty_to_ty call that only checks for (a subset of) valid const types, when the direct type lookup fails. It's kind of hacky, so I understand if you don't want to take this as is. I'd need a little mentoring to get this into better shape, as figuring out the proper fix has been a little daunting. I'm also happy if someone else wants to pick this up and run with it.
This closes 5900 and 9942, but only moves the goalposts a little on 10618, which now falls over in a later phase of the compiler.