[PATCH] D143128: [-Wunsafe-buffer-usage][WIP] Fix-Its transforming `&DRE[any]` to `DRE.data() + any`
Ziqing Luo via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Feb 6 15:05:55 PST 2023
ziqingluo-90 added inline comments.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:508
+ hasUnaryOperand(arraySubscriptExpr(
+ hasBase(ignoringParenImpCasts(declRefExpr())))))
+ .bind(UPCAddressofArraySubscriptTag)))));
----------------
jkorous wrote:
> I am wondering what will happen in the weird corner-case of `&5[ptr]` - I feel the Fix-It we produce would be incorrect.
>
> Here's a suggestion - we could use `hasLHS` instead of `hasBase` here and add a FIXME that when we find the time we should also properly support the corner-case. That would be a pretty low-priority though - we definitely have more important patterns to support first.
>
> WDYT?
>
I'm not sure if I understand your concern. For `&5[ptr]`, we will generate a fix-it `ptr.data() + 5` in cases `ptr` is assigned a `span` strategy. It is same as the case of `&ptr[5]`.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:810
+ case Strategy::Kind::Span:
+ return fixUPCAddressofArraySubscriptWithSpan(Node);
+ case Strategy::Kind::Wontfix:
----------------
jkorous wrote:
> Since we use `std::nullopt` in `getFixits` to signal errors - we should either use the same strategy in `fixUPCAddressofArraySubscriptWithSpan` or translate the empty return value from it to `nullopt` here.
> (FWIWI I am leaning towards the former.)
> Forwarding the empty Fix-It would be incorrect.
>
>
Oh, that's a bug I made! Thank you for finding it for me.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143128/new/
https://reviews.llvm.org/D143128
More information about the cfe-commits
mailing list