some additional need_type_info.rs cleanup#97703
Conversation
|
Some changes occured in need_type_info.rs cc @lcnr |
|
r? @wesleywiser (rust-highfive has picked a reviewer for you, use r? to override) |
There was a problem hiding this comment.
Moving slowly but surely! (you can take a look at my work in progress there: https://github.com/rust-lang/rust/compare/master...GuillaumeGomez:rustc-middle-ty-alias?expand=1)
There was a problem hiding this comment.
Is turning this into a warn instead of bug intended?
There was a problem hiding this comment.
yeah, don't want to ICE here and getting it wrong only worsens diagnostics.
feels safer than using a bug!
There was a problem hiding this comment.
What about using delay_span_bug instead?
There was a problem hiding this comment.
delay_span_bug won't ever trigger, considering that we're always emitting an error
There was a problem hiding this comment.
It does protect against a subsequent refactor making us hit this codepath without an error being emitted.
There was a problem hiding this comment.
the FindInferSourceVisitor should not be relevant to whether we emit an error. I do not think this is an issue. Or at least I think the guard for that shouldn't be at this point. We currently guard against it by fn emit_inference_failure_err returning a DiagnosticBuilder<'tcx, ErrorGuaranteed>.
compiler/rustc_infer/src/infer/error_reporting/need_type_info.rs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Let's make it clear in the naming and docs that this is only meant for use in diagnostics code.
There was a problem hiding this comment.
added a comment, don't want to make these method names even more unwieldy 😅
|
r? @estebank |
it feels arbitrary to have `Ty` and `Const` directly in that module and to not have `GenericArg` and `GenericArgKind` there. Writing `ty::GenericArg` can also feel clearer than importing it. Using `ty::subst::GenericArg` however is ugly.
| }) | ||
| .count(); | ||
| let generic_args = | ||
| &generics.own_substs(substs)[generics.own_counts().lifetimes..][..num_args]; |
There was a problem hiding this comment.
that caused the ICE in #97698 (comment). Writing big PRs sure is difficult 😅
|
@bors r+ |
|
📌 Commit d6b28f3 has been approved by |
Rollup of 6 pull requests Successful merges: - rust-lang#96868 (Stabilize explicit_generic_args_with_impl_trait) - rust-lang#97703 (some additional `need_type_info.rs` cleanup) - rust-lang#97812 (Suggest to swap a struct and a trait in trait impls) - rust-lang#97958 (ExitStatus docs fixups) - rust-lang#97967 (Mention `infer::Trace` methods on `infer::At` methods' docs) - rust-lang#97972 (Update #[doc(html_playground_url)] documentation to mention what the request will be) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
also fixes #97698, fixes #97806
cc @estebank