ziqingluo-90 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.
----------------
NoQ wrote:
> 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 🤔
sorry I didn't think it through when I reply the comments. You are right.
Fixables and variable declarations should be irrelevant to opt-out regions.
I'll rebase this patch w.r.t. https://reviews.llvm.org/D139737 so that we will
have 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
