njames93 added a comment.
In D76054#1935533 <https://reviews.llvm.org/D76054#1935533>, @ymandel wrote:
> Thanks for expanding the description, including the helpful example. I'm not
> sure, though, that this is the "right" behavior, at least not always. Worse,
> I'm not sure there is a single "right" behavior. I can easily imagine a tidy
> that matches multiple times in the same TU and tries, therefore, to apply the
> same fix multiple times, even though it wants at most one (and possibly an
> error). The major problem is that character-level (versus AST level) edits
> simply don't compose. So, multiple edits on a file are always suspect. Could
> you also give an example (in terms of code changes, not YAML) of why this
> comes up? As in, are we sure that the problem lies in the algorithm here,
> rather than in the phrasing of the transformation itself?
>
> That said, based on some of your linked bugs, it sounds like your change
> would make the behavior here consistent with the behavior in another
> situation (when clang-tidy applies the edits directly rather than outputting
> to YAML?). Consistency could be a strong argument in favor of this change.
clang-tidy does a good job of filtering out conflicting fix-its so duplicated
insertions are going to be intentional.
The example yaml is generated from running is running the
readability-braces-around-statements check on this code(offsets dont match due
to formatting)
void f() {
if (1)
if (2) return;
}
Both if statements have the same end location which is where the check will
insert a `}`.
This leads to 2 insertion fix its that are the same location and same text.
When clang-tidy applies the fix-it there is no issue, but currently
clang-apply-replacements thinks they should be deduplicated.
This ultimately leads to malformed code as 2 `{` are inserted after the if
conditions, but only one `}` is inserted at the end.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76054/new/
https://reviews.llvm.org/D76054
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
