ASDenysPetrov added inline comments.
================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:229-233 + // We want to keep the following invariant at all times: + // ---[ First ------> + // -----[ Second ---> + if (First->From() > Second->From()) + swapIterators(First, FirstEnd, Second, SecondEnd); ---------------- martong wrote: > I am not sure about this, but perhaps we could put this `swapIterators` call > right at the beginning of the nested `while` loop (L243). That would > eliminate the need to call it again at the end of the second while loop. I'm afraid, it is not the case. Every loop needs its own `swapIterators`. As you can see, `swapIterators` in the nested loop invokes not always because of `break;` in the middle. Otherwise, it would invokes each time. But I checked your suggestion. It doesn't work. ================ Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:243 + // Loop where the invariant holds. + while (true) { + // Skip all enclosed ranges. ---------------- martong wrote: > So, this loop is about to merge conjunct Ranges. The first time when we find > disjoint Ranges then we break out. (Or we return once we reach the end of any > of the RangeSets.) > This makes we wonder, if it would be possible to split this `while` loop into > a lambda named `mergeConjunctRanges` ? I'm not in favor of introducing another level of abstraction. It would divide the flow/comments and could reduce readability and performance which is crucial here. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99797/new/ https://reviews.llvm.org/D99797 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits