[PATCH] D74473: [analyzer] StdLibraryFunctionsChecker: Use platform dependent EOF and UCharMax

[Gabor Marton via Phabricator via cfe-commits]

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


================
Comment at: clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp:571
+        .Case(
+            {ReturnValueCondition(WithinRange, {{EOFv, EOFv}, {0, UCharMax}})});
   };
----------------
Szelethus wrote:
> Huh, what's happening here exactly? Doesn't `Range` take 2 `RangeInt`s as ctor arguments? What does `{EOFv, EOFv}, {0, UCharMax}` mean in this context?
`Range` is a convenience function that creates a `IntRangeVector` from the two params. This way we can easily create a list of ranges from two ints. Range(-1, 255) results a list of ranges with one list: {{-1, 255}}.
Now, because EOF's value can be less than the minimum value of the next range we have to explicitly enlist the ranges.
E.g. if EOF is -2 then we will have a list of {-2,-2} and {0, 255}.


================
Comment at: clang/lib/StaticAnalyzer/Core/CheckerHelpers.cpp:114-117
+  const auto MacroIt = llvm::find_if(
+      PP.macros(), [&](const auto &K) { return K.first->getName() == Macro; });
+  if (MacroIt == PP.macro_end())
+    return llvm::None;
----------------
Szelethus wrote:
> This seems a bit clunky even for the `Preprocessor` -- how about
> 
> ```lang=c++
> const auto *MacroII = PP.getIdentifierInfo(Macro);
> if (!MacroII)
>   return;
> const MacroInfo *MI = PP.getMacroInfo(MacroII);
> assert(MI);
> ```
Ok, but we cannot assert on `MI`, because there may be cases when the macro is not defined in a TU. In that case we should just return with None.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74473