[all-commits] [llvm/llvm-project] 6538b4: [clang-apply-replacements] No longer deduplucates ...

Nathan James via All-commits all-commits at lists.llvm.org
Wed Mar 25 02:36:45 PDT 2020


  Branch: refs/heads/master
  Home:   https://github.com/llvm/llvm-project
  Commit: 6538b4393dc3e7df9fee2b07eba148d4cf603a24
      https://github.com/llvm/llvm-project/commit/6538b4393dc3e7df9fee2b07eba148d4cf603a24
  Author: Nathan James <n.james93 at hotmail.co.uk>
  Date:   2020-03-25 (Wed, 25 Mar 2020)

  Changed paths:
    M clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
    A clang-tools-extra/test/clang-apply-replacements/Inputs/identical-in-TU/file1.yaml
    A clang-tools-extra/test/clang-apply-replacements/Inputs/identical-in-TU/file2.yaml
    A clang-tools-extra/test/clang-apply-replacements/Inputs/identical-in-TU/identical-in-TU.cpp
    A clang-tools-extra/test/clang-apply-replacements/identical-in-TU.cpp

  Log Message:
  -----------
  [clang-apply-replacements] No longer deduplucates replacements from the same TU

Summary:
clang-apply-replacements currently deduplicates all diagnostic replacements. However if you get a duplicated replacement from one TU then its expected that it should not be deduplicated. This goes some way to solving [[ https://bugs.llvm.org/show_bug.cgi?id=45150 | export-fixes to yaml adds extra newlines and breaks offsets. ]]

Take this example yaml.
```
---
MainSourceFile:  '/home/nathan/test/test.cpp'
Diagnostics:
  - DiagnosticName:  readability-braces-around-statements
    DiagnosticMessage:
      Message:         statement should be inside braces
      FilePath:        '/home/nathan/test/test.cpp'
      FileOffset:      14
      Replacements:
        - FilePath:        '/home/nathan/test/test.cpp'
          Offset:          14
          Length:          0
          ReplacementText: ' {'
        - FilePath:        '/home/nathan/test/test.cpp'
          Offset:          28
          Length:          0
          ReplacementText: '

}'
  - DiagnosticName:  readability-braces-around-statements
    DiagnosticMessage:
      Message:         statement should be inside braces
      FilePath:        '/home/nathan/test/test.cpp'
      FileOffset:      20
      Replacements:
        - FilePath:        '/home/nathan/test/test.cpp'
          Offset:          20
          Length:          0
          ReplacementText: ' {'
        - FilePath:        '/home/nathan/test/test.cpp'
          Offset:          28
          Length:          0
          ReplacementText: '

}'
...```

The current behaviour is to deduplicate the text insertions at Offset 28 and only apply one of the replacements.
However as both of these replacements came from the same translation unit we can be confident they were both meant to be applied together
The new behaviour won't deduplicate the text insertion and instead insert both of the replacements.
If the duplicate replacement is found inside different translation units (from a header file change perhaps) then they will still be deduplicated as before.

Reviewers: aaron.ballman, gribozavr2, klimek, ymandel

Reviewed By: ymandel

Subscribers: ymandel, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D76054




More information about the All-commits mailing list