Type fallback refactorings#159026
Conversation
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Type fallback refactorings
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (6a25a06): comparison URL. Overall result: ❌✅ regressions and improvements - please read:Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf. Next, please: If you can, justify the regressions found in this try perf run in writing along with @bors rollup=never Instruction countOur most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.
Max RSS (memory usage)Results (primary 3.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary -5.3%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (primary 0.1%, secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 487.967s -> 488.861s (0.18%) |
68f06e3 to
8992f0e
Compare
|
I think the perf regression is noise. |
| /// - Unconstrained floats are replaced with `f64`, except when there is a trait predicate | ||
| /// `f32: From<{float}>`, in which case `f32` is used as the fallback instead. |
There was a problem hiding this comment.
can you change this comment to name the FCW we emit here so that we update this comment when removing the fallback
| if self.root_var(vid) != vid { | ||
| return None; | ||
| } | ||
| match self.probe(vid) { | ||
| TypeVariableValue::Unknown { .. } => Some(vid), | ||
| TypeVariableValue::Known { .. } => None, | ||
| } |
There was a problem hiding this comment.
you can merge this with ena version 0.14.4, see #158447
There's an operation which does the "is known" and "get root var" with one access
| .map(|&ty_id| self.shallow_resolve(Ty::new_var(self.tcx, ty_id))) | ||
| .filter_map(|ty| ty.ty_vid()) | ||
| .map(|vid| self.root_var(vid)) |
There was a problem hiding this comment.
that should also be a probe_with_root_vid i think? seems like a very cumbersome way to get the root vid of a ty var
| "calculate_diverging_fallback: root_vid={:?} reaches {:?}", | ||
| let unsafe_infer_vars = OnceCell::new(); | ||
| for &root_vid in &diverging_root_vids { | ||
| self.lint_never_type_fallback_flowing_into_unsafe_code( |
There was a problem hiding this comment.
why are we using a OnceCell here instead of a if !diverging_root_vids.is_empty() around the whole block here?
|
unsure about the last commit after applying my other review comments, but r=me after nits on the other changes |
r? types
Tybefore matching them backcc @lcnr