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

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

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.



================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:406
+    const GadgetFix *End = FixIts + NumFixIts;
+    std::vector<GadgetFix> Result;
+    const BitVector &BV = ConflictingFixIts;
----------------
`Result` is unused here!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141338