martong added inline comments.
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:194
+ const ProgramPointTag *Tag = nullptr) {
+ // Say this 3 times fast.
+ State = State ? State : getState();
----------------
I like the joke, but this comment does not have any value, could you please
remove?
================
Comment at:
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h:195
+ // Say this 3 times fast.
+ State = State ? State : getState();
+ addTransition(State, generateSink(State, getPredecessor()));
----------------
balazske wrote:
> ```
> if (!State)
> State = getState();
> ```
> is better? (I put the same comment into some (other?) patch already but maybe
> it disappeared?)
+1 for balazske's suggestion.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:1744-1745
+ // NOTE: There is a philosophical question to be answered when we detect a
+ // bug, but the checker under whose name we would emit the error is disabled.
+ // Generally speaking, the MallocChecker family is an integral part of the
----------------
This sentence is too bloated, I'd rather remove it.
AFAIU, the general rule of thumb is that if we have found a bug then we
terminate further analysis, period. This is independent of whether we emit the
warning or not, that actually depends on whether the corresponding subchecker
is enabled or not.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp:2024
+ if (!ChecksEnabled[CK_MallocChecker] && !ChecksEnabled[CK_NewDeleteChecker])
{
+ C.addSink();
return;
----------------
This seems to be inverse logic to me.
I'd expect that in a function called `Report...` we do stuff that is related to
reporting only. That is why I think it would be better to have the condition
and addSink before calling `Report...`. That way reporting and modeling would
be even more separated.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77474/new/
https://reviews.llvm.org/D77474
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
