[PATCH] D86531: [analyzer][StdLibraryFunctionsChecker][NFC] Use Optionals throughout the summary API

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1006
+        return IntRangeVector{std::pair<RangeInt, RangeInt>{b, *e}};
+      return IntRangeVector{};
+    }
----------------
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.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1321
 
-    Optional<QualType> Mode_tTy = lookupType("mode_t", ACtx);
+    Optional<QualType> Mode_tTy = lookupTy("mode_t");
 
----------------
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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86531/new/

https://reviews.llvm.org/D86531