[PATCH] D70144: clang-tidy: modernize-use-equals-default avoid adding redundant semicolons

[Conrad Poelman via Phabricator via cfe-commits]

poelmanc added a comment.

> ! In D70144#1745583 <https://reviews.llvm.org/D70144#1745583>, @JonasToth wrote:
> 
>> ! In D70144#1745532 <https://reviews.llvm.org/D70144#1745532>, @malcolm.parsons wrote:
> 
> Should `clang::format::cleanupAroundReplacements()` handle this instead?

My initial attempt did not go well. I thought perhaps adding:

  cleanupLeft(Line->First, tok::semi, tok::semi);

to clang/lib/Format/Format.cpp:1491 might do the trick. Stepping through that in the debugger shows that `cleanupPair` iterates over tokens on affected lines over the affected range. But after the newly added `default` token and subsequent `semi` token comes a nullptr - I could not see how to peek past the `default;` to see what else is on the line.

There's a comment admitting that this needs some architectural work:

  // FIXME: in the current implementation the granularity of affected range
  // is an annotated line. However, this is not sufficient. Furthermore,
  // redundant code introduced by replacements does not necessarily
  // intercept with ranges of replacements that result in the redundancy.
  // To determine if some redundant code is actually introduced by
  // replacements(e.g. deletions), we need to come up with a more
  // sophisticated way of computing affected ranges.

Agreed. Even if we could see all the tokens on a line, it seems the test case I added to this patch at clang-tidy/checkers/modernize-use-equals-default.cpp:50, where the redundant semicolon is on the next line, could never be addressed given the current architecture. Changing the TokenAnalyzer::analyze() interface would affect JavaScriptRequoter, ObjCHeaderStyleGuesser, JavaScriptImportSorter, Formatter, and others.

Thoughts on how to proceed?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70144