[PATCH] D122841: [analyzer] Add option for AddrSpace in core.NullDereference check

[Vince Bridgers via Phabricator via cfe-commits]

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