[PATCH] D157441: [-Wunsafe-buffer-usage] Use `Strategy` to determine whether to fix a parameter

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D157441#4575220 <https://reviews.llvm.org/D157441#4575220>, @ziqingluo-90 wrote:

>>> Extend PointerAssignmentGadget and PointerInitGadget to generate fix-its for cases where the left-hand side 
>>> has won't fix strategy and the right-hand side has std::span strategy.
>>
>> Hmm, is this intertwined with other changes, or is this completely separate? Also my head appears to be blank;
>
> The two `FixableGadget`s previously gave incorrect fix-its for the cases where only RHS need fix.  After the change made to `Strategy`, they returned `std::nullopt` as the case was not implemented.
> So to avoid regression, I made the extension to the two Gadgets.
>
>> did we reach any conclusion about the nature of "single" RHS-es and usefulness of annotating them as such?
>> Which could be an approach that would encourage people to propagate more bounds in the backwards direction by default.
>
> Can you elaborate the question a bit more?

If previously we were emitting an incorrect fixit and now we're not emitting any fixit, then I think it's a very good improvement, not a regression 🤔

Uhh, I meant LHSes, sorry 😅

Anyway, the point was that it's hard to tell whether `.data()` is the right fix, so I think we wanted to hold off implementing it, because we probably want to do something better instead in most cases. Like, implement a strategy discovery procedure that would give us a better idea of //how exactly// is the LHS pointer used safely. Then we might emit different fixes based on that. So I suspect that it should be a separate effort, so it probably makes sense to separate it into a different patch and think more about it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157441