[PATCH] D82288: [analyzer][StdLibraryFunctionsChecker] Add POSIX file handling functions

[Gabor Marton via Phabricator via cfe-commits]

martong marked 8 inline comments as done.
martong added inline comments.


================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:352-357
     CmdLineOption<Boolean,
                   "DisplayLoadedSummaries",
                   "If set to true, the checker displays the found summaries "
                   "for the given translation unit.",
                   "false",
+                  Released>,
----------------
Szelethus wrote:
> D78118#inline-723679 Please mark this `Hidden`.
Ok, it missed my attention about that back then.


================
Comment at: clang/include/clang/StaticAnalyzer/Checkers/Checkers.td:358-363
+    CmdLineOption<Boolean,
+                  "ModelPOSIX",
+                  "If set to true, the checker models functions from the "
+                  "POSIX standard.",
+                  "false",
                   Released>
----------------
Szelethus wrote:
> I'm fine with not hiding this, but as a user, why would I never not enable it? If the reasoning is that this isn't ready quite yet, change `Released` to `InAlpha`. Otherwise, either mark this `Hidden` or give a user friendly explanation as to what is there to gain/lose by tinkering with this option.
Yes, thanks, I wanted to make it similar to alpha checkers, now I know it is `InAlpha` that I need.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:740
   // we have a TypedefDecl with the name 'FILE'.
-  for (Decl *D : LookupRes) {
+  for (Decl *D : LookupRes)
     if (auto *TD = dyn_cast<TypedefNameDecl>(D))
----------------
gamesh411 wrote:
> I presume that typically there are only a handful of `Decl`s in LookupRes, so it not worth the complexity of using `std::algorithm`s to sort/partition/find.
There may be many `Decl`s in the lookup result, but I don't know based on which property would it make sense to sort them. They are essentially stored in the order of creation, so as the parser reads an upcoming Decl.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:1591
+
+    Optional<RangeInt> Off_tMax;
+    if (Off_tTy) {
----------------
gamesh411 wrote:
> If `Off_tMax` is only used in the if block that follows, consider moving it's declaration inside.
Yeah, good catch, thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82288