balazske marked 3 inline comments as done.
balazske added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:63
+ const BinaryOperator *BinOp,
+ const llvm::APSInt *KnownValue,
+ QualType RetTy,
----------------
Szelethus wrote:
> This is the value we're checking //against//, I think the name `KnownValue`
> doesn't convey this.
Renamed to `ValueToTestAgainst`.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:79-80
+ bool ChildIsLHS) const override {
+ if (!KnownValue)
+ return false;
+
----------------
balazske wrote:
> Szelethus wrote:
> > So if we failed to get retrieve the value we're checking against, we
> > automatically assume that its not a proper check? Shouldn't we be
> > conservative here? What if that value is something like `getEOFValue()`?
> This case needs attention, now it is detected as failure (maybe the function
> can return `Optional<bool>` to indicate a non-determinable case).
This case is now not error.
A test is added for this case.
```
int R = fputs("str", file());
if (R == getEOFValue())
return;
```
This is no warning now (but `getEOFValue` can return anything).
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:83
+ bool KnownNull = KnownValue->isNullValue();
+ bool KnownEOF = ((*KnownValue) == BVF.getValue(-1, RetTy));
+
----------------
Szelethus wrote:
> We have a smarter EOF getter thanks to @martong, don't we?
`tryExpandAsInteger` is used now.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/ErrorReturnChecker.cpp:220-223
+/// Information about a specific function call that has an error return code to
+/// check. This data is stored in a map and indexed by the SymbolRef that
stands
+/// for the result of the function call.
+struct CalledFunctionData {
----------------
Szelethus wrote:
> The comments leave me to believe that the correct class name would be
> `FunctionCallData`, right? Because `FnInfo` does the job of describing the
> called function, and this one deals with the actual call.
`CalledFunctionData` was renamed to `FunctionCallData` and `CFD` variables to
`Call`. It looks better now.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72705/new/
https://reviews.llvm.org/D72705
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
