[PATCH] D54017: [analyzer] NullabilityChecker: Invariant violation should only be triggered for symbols.

[Umann Kristóf via Phabricator via cfe-commits]

Szelethus added a comment.

Thanks for summarizing the problem so well in the summary! I should start doing that too.



================
Comment at: lib/StaticAnalyzer/Checkers/NullabilityChecker.cpp:343-346
+  // There should have originally been a symbol. If it was a concrete value
+  // to begin with, the violation should have been handled immediately
+  // rather than delayed until constraints are updated. Why "symbol" rather than
+  // "region"? Because only symbolic regions can be null.
----------------
To me, "should've been handled" sounds like that could be translated to an assert.

Also, this doc seems confusing to me, but I might be wrong. You referenced the term "symbol" a couple types, but I don't see `SymbolVal` anywhere. Could you reword it a bit?


================
Comment at: test/Analysis/nullability-arc.mm:1-2
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,nullability -analyzer-output=text -verify %s
+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core,nullability -analyzer-output=text -verify %s -fobjc-arc
+
----------------
Let's organize this into multiple lines!

```
// RUN: %clang_analyze_cc1 -w -verify %s -fobjc-arc \
// RUN:   -analyzer-checker=core,nullability -analyzer-output=text
```


================
Comment at: test/Analysis/nullability.mm:3-4
 // RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=true -DNOSYSTEMHEADERS=1 -verify %s
+// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -DNOSYSTEMHEADERS=0 -verify %s -fobjc-arc
+// RUN: %clang_analyze_cc1 -fblocks -analyzer-checker=core,nullability.NullPassedToNonnull,nullability.NullReturnedFromNonnull,nullability.NullablePassedToNonnull,nullability.NullableReturnedFromNonnull,nullability.NullableDereferenced -analyzer-config nullability:NoDiagnoseCallsToSystemHeaders=true -DNOSYSTEMHEADERS=1 -verify %s -fobjc-arc
 
----------------
These too. Especially these :D


Repository:
  rC Clang

https://reviews.llvm.org/D54017