ASDenysPetrov added a comment.
In D99797#3059203 <https://reviews.llvm.org/D99797#3059203>, @steakhal wrote:
> PS: the test coverage is outstanding!
Thank you for this analysis.
================
Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:149
+
+RangeSet RangeSet::Factory::unite(RangeSet Original, llvm::APSInt Point) {
+ return unite(Original, Range(ValueFactory.getValue(Point)));
----------------
steakhal wrote:
> Why do you take `APSInt`s by value? Generally, we take them by reference.
I want to send a message to the caller that he can pass an arbitrary **APSInt**
without warrying about making it permanent (aka stored by the Factory). But we
can revise this contract and carry this responsibility to a caller.
================
Comment at: clang/unittests/StaticAnalyzer/RangeSetTest.cpp:81
const llvm::APSInt &from(BaseType X) {
- llvm::APSInt Dummy = Base;
- Dummy = X;
- return BVF.getValue(Dummy);
+ static llvm::APSInt Base{sizeof(BaseType) * 8,
+ std::is_unsigned<BaseType>::value};
----------------
steakhal wrote:
> Shouldn't you use `sizeof(BaseType) * CHAR_BIT` instead?
Agree. It's better to avoid magic numbers. I'll fix.
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
