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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to