simplify setup_constraining_predicates, and note it is potentially cubic #146980
Conversation
rustbot
added
S-waiting-on-review
|
rustbot has assigned @jdonszelmann. Use |
| // FIXME(hkBst): the big-O bound above would be accurate for the number | ||
| // of calls to `parameters_for`, which itself is some O(complexity of type). | ||
| // That would make this potentially cubic instead of merely quadratic... | ||
| // ...unless we cache those `parameters_for` calls. |
There was a problem hiding this comment.
which we don't currently, it seems
|
what is the goal of this PR? is the plan to modify this later not to be cubic? Though I think the code is indeed slightly cleaner I don't quite see that it's clearly an improvement over what was already there. In some sense, the largest code change here is combining three ifs into a single if-let which imo isn't necessarily clearer than the continue, especially the block-in-condition you have there |
|
looking at it more, I'm less and less convinced this is a real improvement except maybe the note that it could be cubic. Unless you're working on fixing that I'm not sure this is such a useful change. |
|
@rustbot author |
rustbot
added
S-waiting-on-author
I would, but I'm not familiar enough with the compiler to attempt it yet, I think.
Well, there are currently 3 continues inside this while loop, so who knows what it does? The proposed code makes it clear that it's just a complicated way to write if-conditions. The block-in-condition can easily be changed to not require the block, if you think that would help. It's just inherited like this from the current code. |
|
Hmm, well I think that right now the old version is equally clear if not clearer. A large if let chain with a block in it isn't much better than some continues. If you can change it so it's actually clear what the codepaths are (maybe name some variables?) I think it'd be much better |
|
The big difference between a loop body with 3 continues and one with none, is that in one case you don't know how much of the loop body is executed for each iteration, and for the other case you know that the whole loop body is executed each time. One very concrete benefit: in the current code line 215 and 219-222 are separated by an |
|
I've now flattened the conditional block. @rustbot ready |
rustbot
added
S-waiting-on-review
|
@bors r+ rollup |
bors
added
S-waiting-on-bors
Rollup of 11 pull requests Successful merges: - #146918 (add regression test) - #146980 (simplify setup_constraining_predicates, and note it is potentially cubic) - #147170 (compiletest: Pass around `DirectiveLine` instead of bare strings) - #147180 (add tests) - #147188 (Remove usage of `compiletest-use-stage0-libtest` from CI) - #147189 (Replace `rustc_span::Span` with a stripped down version for librustdoc's highlighter) - #147199 (remove outdated comment in (inner) `InferCtxt`) - #147200 (Fix autodiff empty ret regression) - #147209 (Remove `no-remap-src-base` from tests) - #147213 (Fix broken STD build for ESP-IDF) - #147217 (Don't create a top-level `true` directory when running UI tests) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #146980 - hkBst:hir-analysis-1, r=jdonszelmann simplify setup_constraining_predicates, and note it is potentially cubic
No description provided.