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
