[PATCH] D141338: [WIP][-Wunsafe-buffer-usage] Filter out conflicting fix-its

Ziqing Luo via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 11 17:09:50 PST 2023


ziqingluo-90 added a comment.

In D141338#4042050 <https://reviews.llvm.org/D141338#4042050>, @NoQ wrote:

> Looks awesome!
>
> I'm worried that the test is going to rapidly become stale as you develop fixits for `DeclStmt` further. It might make sense to write some //unittests// for this class directly, to put into `clang/unittest/Analysis/...`. Then you can create some fixits defined by hand, without invoking UnsafeBufferUsage analysis, and feed them to this class directly and verify the results.
>
> Also let's explain motivation a bit more. Normally compiler warnings don't need such machine because the fixits they come with are very simple and hand-crafted. However, unsafe buffer usage analysis is a large emergent behavior machine that may fix different parts of user code, and different parts of the machine that can produce parts of such fixits aren't necessarily aware of each other. In some cases no progress can be made without making sure these parts of the machine talk to each other. However, every time the machine does emit individual fixits, they're actually correct; conflicts between such fixits are entirely a representation problem (we're unable to present overlapping fixits to the user because this requires us to resolve the merge conflict), not an underlying correctness problem. So @ziqingluo-90 is implementing a bailout for the situation when the fixits were perfectly correct in isolation but can't be properly displayed to the user due to merge conflicts between them. This makes it possible for such "merge conflicts" detection to be purely SourceRange-based, so it doesn't need to take the details of the underlying AST into account.

Thanks for adding the motivation here!

I updated the revision by getting rid of things we don't need for now.   
At this point, we just need to tell if there is any conflict in the fix-its generated for one variable (including variable declaration fix-its and variable usage fix-its).  If there is any,  we do NOT emit any fix-it for that variable.

At some point later, we may want to know the exact conflicting subsets.  That will be another patch.


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

https://reviews.llvm.org/D141338



More information about the cfe-commits mailing list