ASDenysPetrov added inline comments.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h:103-110
+ enum class CompareResult {
+ unknown,
+ identical,
+ less,
+ less_equal,
+ greater,
+ greater_equal
----------------
Can we use `Optional<BinaryOperatorKind>` instead, to reduce similar enums? Or
you want to separate the meaning in a such way?
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:30-33
+ const int LMaxRMin =
+ llvm::APSInt::compareValues(getMaxValue(), other.getMinValue());
+ const int LMinRMax =
+ llvm::APSInt::compareValues(getMinValue(), other.getMaxValue());
----------------
As you use here `getMaxValue` twice which is not O(1), it'd better to use a
buffer variable.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:106-113
+const llvm::APSInt &RangeSet::getMaxValue() const {
+ assert(!isEmpty());
+ auto last = ranges.begin();
+ for (auto it = std::next(ranges.begin()); it != ranges.end(); ++it)
+ last = it;
+ return last->To();
+}
----------------
Can we improve `llvm::ImmutableSet` this to make `getMaxValue` complexity O(1)?
================
Comment at: clang/test/Analysis/constraint-manager-sym-sym.c:181
+// {[0,1],[20,20]} and {[9,9],[11,42],[44,44]}
+void test_range18(int l, int r) {
+ assert((9 <= r && r <= 9) || (11 <= r && r <= 42) || (44 <= r && r <= 44));
----------------
Could you add some tests for `unsigned, unsigned` and `signed, unsigned`?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77792/new/
https://reviews.llvm.org/D77792
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
