vabridgers planned changes to this revision.
vabridgers added a comment.
Thanks for the the comments. I'll make the changes and resubmit. Please let me
know your preferences for the test case (default vs pinned x86 or same).
Best!
================
Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:331
+ auto *Chk = mgr.registerChecker<DereferenceChecker>();
+ // auto *Chk = mgr.getChecker<DereferenceChecker>();
+ Chk->DetectAllNullDereferences =
----------------
steakhal wrote:
> steakhal wrote:
> > dead-code?
> This still looks dead.
arg, missed that. I'll clean this up.
================
Comment at: clang/lib/StaticAnalyzer/Checkers/DereferenceChecker.cpp:56
+
+ bool SuppressAddressSpaces = false;
};
----------------
steakhal wrote:
> If I'm correct, we tend to use the 'DefaultBool' instead of plain old bools
> for checker options. Could you please check if this is the case and
> consolidate this?
I'll address. FYI, looks like I chose the red pill when I found an example to
go by :/ Looks like there's opportunity for some cleanup NFC type of patches.
I'll pick these up in a separate patch.
```
120 namespace {
121 class DeadStoresChecker : public Checker<check::ASTCodeBody> {
122 public:
123 bool ShowFixIts = false;
124 bool WarnForDeadNestedAssignments = true;
125
126 void checkASTCodeBody(const Decl *D, AnalysisManager &Mgr,
127 BugReporter &BR) const;
128 };
```
================
Comment at: clang/test/Analysis/cast-value-notes.cpp:20
+// RUN: -verify -DDEFAULT_TRIPLE %s 2>&1 | FileCheck %s\
+// RUN: -check-prefix=DEFAULT-CHECK
+
----------------
steakhal wrote:
> Nit: the default triple is the host/native triple, but you did actually pin
> it to x86. Hence it might not be the default on other platforms.
Yeah, the result is the same (as far as what the test code would look like).
Not sure what your preferences are here.
Would you prefer to see separate cases (default and pinned x86), or perhaps the
macro names renamed to something that means both (instead of implying default)?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122841/new/
https://reviews.llvm.org/D122841
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
