NoQ added inline comments.

================
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.
----------------
ziqingluo-90 wrote:
> 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.
> do you mean that a variable may still get fixed even if its declaration is in 
> an opt-out zone?

Yes I think that's the case. We do have tests about this right?:
```lang=c++
#pragma clang unsafe_buffer_usage begin
  ...
  int *p3 = new int[10]; // expected-warning{{'p3' is an unsafe pointer used 
for buffer access}}

#pragma clang unsafe_buffer_usage end
  ...
  p3[5]; //expected-note{{used in buffer access here}}
```
And I think it's safe to assume that every time we emit a warning, we also want 
to emit a fixit.

If only we had any fixits implemented, this code would have been much easier to 
refactor because we'd have some actual tests covering it 🤔


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