martong added inline comments.
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1006
+ return IntRangeVector{std::pair<RangeInt, RangeInt>{b, *e}};
+ return IntRangeVector{};
+ }
----------------
balazske wrote:
> This return of empty vector and possibility of adding empty vector to range
> constraint is a new thing, probably it is better to check at create of range
> constraint (or other place) for empty range (in this case the summary could
> be made invalid)? But this occurs probably never because the matching type
> (of the max value) should be used at the same function and if the type is
> there the max value should be too.
Alright, I added an assert to RangeConstraint::getRanges:
```
const IntRangeVector &getRanges() const {
+ // When using max values for a type, the type normally should be part of
+ // the signature. Thus we must had looked up previously the type. If the
+ // type is not found then the range would be empty, but then the summary
+ // should be invalid too.
+ assert(Args.size() && "Empty range is meaningless");
return Args;
}
```
================
Comment at:
clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1321
- Optional<QualType> Mode_tTy = lookupType("mode_t", ACtx);
+ Optional<QualType> Mode_tTy = lookupTy("mode_t");
----------------
balazske wrote:
> martong wrote:
> > balazske wrote:
> > > It is better to get every type at the start before adding functions, or
> > > group the functions and get some types at the start of these groups but
> > > mark the groups at least with comments.
> > Well, with looked-up types I followed the usual convention to define a
> > variable right before using it. This means that we lookup a type just
> > before we try to add the function which first uses that type.
> >
> > However, builtin types are defined at the beginning, because they are used
> > very often.
> I still like it better if all the type variables are created at one place
> (can be more easy to maintain if order changes and we have one block of types
> and one of functions) but this is no reason to block this change.
I see your point, still I'd keep this way, because I this way the functions and
the types they use are close to each other in the source.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86531/new/
https://reviews.llvm.org/D86531
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
