fulfill: dedup newly added obligations#145714
Conversation
|
|
|
Some changes occurred to the core trait solver cc @rust-lang/initiative-trait-system-refactor |
|
@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.
fulfill: dedup newly added obligations
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (f5323d6): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please do so in sufficient 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 0.6%, secondary -1.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 2.5%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 471.281s -> 471.144s (-0.03%) |
|
|
||
| #[derive(Default, Debug)] | ||
| struct ObligationStorage<'tcx> { | ||
| dedup: FxHashSet<(ty::ParamEnv<'tcx>, ty::Predicate<'tcx>)>, |
There was a problem hiding this comment.
What happens if ?0: Tr and ?1: Tr both get constrained to i32: Tr? Do we not dedup in that case?
|
r? BoxyUwU |
| impl<'tcx> ObligationStorage<'tcx> { | ||
| fn register( | ||
| fn register(&mut self, obligation: PredicateObligation<'tcx>) { | ||
| if self.dedup.insert((obligation.param_env, obligation.predicate)) { |
There was a problem hiding this comment.
this dedups stuff with different recursion depths which seems wrong. in general this makes me nervous that we'll have a sort of "cache" that doesnt take into account all the inputs. Why not FxHashSet<PredicateObligation<'tcx>> which is the exact type used for the list of obligations to prove? Is it just because of the cause?
|
☔ The latest upstream changes (presumably #145993) made this pull request unmergeable. Please resolve the merge conflicts. |
r? compiler-errors