[PATCH] D28946: [analyzer] Fix memory space for block-captured static locals.

[Devin Coughlin via Phabricator via cfe-commits]

dcoughlin accepted this revision.
dcoughlin added a comment.
This revision is now accepted and ready to land.

LGTM with some minor comments.



================
Comment at: lib/StaticAnalyzer/Checkers/MacOSXAPIChecker.cpp:98
+    const VarDecl *VD = VR->getDecl();
+    // FIXME: These should have correct namespace and thus should be filtered
+    // out earlier. This branch only fires when we're looking from a block,
----------------
"namespace" --> "memory space"?


================
Comment at: lib/StaticAnalyzer/Core/RegionStore.cpp:1853
+    // FIXME: This is only true when we're starting analysis from main().
+    // We're wasting a lot of coverage here.
     if (isa<StaticGlobalSpaceRegion>(MS))
----------------
If I understand correctly, I think 'losing' is a better word here instead of 'wasting', since we're not spending a finite resource unwisely.


================
Comment at: test/Analysis/dispatch-once.m:116
+  };
+}
----------------
It would be good to add a test that trips up the reverted fix from r292800 with a null pointer false positive. This will make sure that someone who tries to address the FIXME in the same way in the future will detect it.


https://reviews.llvm.org/D28946