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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
