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