ziqingluo-90 added inline comments.

================
Comment at: clang/include/clang/Basic/Diagnostic.h:1040-1043
+  // An ordered sequence of "-Wunsafe-buffer-usage" opt-out regions in one
+  // translation unit. Each region is represented by a pair of start and end
+  // locations.
+  SmallVector<std::pair<SourceLocation, SourceLocation>, 8> 
SafeBufferOptOutMap;
----------------
NoQ wrote:
> Ok, now I no longer see why this data should live in DiagnosticEngine. It's 
> mostly about analysis, right? The pragma simply makes our analysis produce 
> different results, regardless of whether these results are used for producing 
> diagnostics or something else. Maybe let's keep it all in Preprocessor?
make sense to me!


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:543
 #define GADGET(x)                                                              
\
-        x ## Gadget::matcher().bind(#x),
+        allOf(x ## Gadget::matcher().bind(#x), notInSafeBufferOptOut()),
 #include "clang/Analysis/Analyses/UnsafeBufferUsageGadgets.def"
----------------
NoQ wrote:
> This prevents safe fixable gadgets from being found in the opt-out zone. I 
> think this clause should only apply to warning gadgets.
You are right!  Fixables should be found regardless of whether they are in an 
opt-out zone.  A Fixable could later be immediately discarded once we know that 
the variable declaration associated to the Fixable is in an opt-out zone.


================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:551-555
+        allOf(declStmt().bind("any_ds"), notInSafeBufferOptOut())
+        // We match all DREs regardless of whether they are in safe-buffer
+        // opt-out region. Because an unclaimed DRE 'd', regardless of where 
it is,
+        // should prevent a Fixable associated to the same variable as 'd'
+        // from being emitting.
----------------
NoQ wrote:
> I think we should match all DeclStmts as well, because otherwise we may be 
> unable to find the variable to fix.
In case we are unable to find the variable to fix,  it means that the variable 
declaration is in an opt-out zone.  So we don't fix the variable anyway, right?

Or do you mean that a variable may still get fixed even if its declaration is 
in an opt-out zone?   I could imagine it is possible if the variable is 
involved in some assignments that we want to fix.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140179/new/

https://reviews.llvm.org/D140179

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

Reply via email to