[PATCH] D76054: [clang-apply-replacements] No longer deduplucates replacements from the same TU

Nathan James via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 22 17:10:56 PDT 2020


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





More information about the cfe-commits mailing list