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

Conrad Poelman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 19 15:19:00 PST 2019


poelmanc added a subscriber: MyDeveloperDay.
poelmanc added a comment.

Just a quick update, I made some progress addressing the architectural limitation of `AnnotatedLines` and `evaluate()`. I changed the `AnnotatedLines` constructor to also take a `Line *PrevAnnotatedLine` argument and set `Line->First->Prev` to the `Last` of the previous line and `Line->Last->Next` to the `First` of the next line. So now the Tokens remain connected across lines, so individual `TokenAnalyzer` subclasses can easily peek ahead and behind the current changed Line if they wish. Places that previously iterated //e.g.// `while(Tok)` had to be changed to iterate `while(Tok && Tok != Line->Last->Next)` - I probably haven't found all of those places yet.

The good news is, with that architectural change my prior addition of:

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

successfully removed redundant semicolons after `default`, even when the next semicolon was on the next line and/or separated by comments!

The bad news is this change also removed consecutive semicolons in //e.g.//:

  for(<possible stuff
      here> ;; <maybe more here>)

which breaks some existing tests. I believe @MyDeveloperDay had thoughts on how to avoid replacing those meaningful consecutive semicolons? We have only tokens rather than an AST. We could search backwards for a `for` token, but I don't know how far backwards we would want to search to determine whether **this particular** `;;` is valid.

In D70144#1748881 <https://reviews.llvm.org/D70144#1748881>, @JonasToth wrote:

> Hmm. I think this is fine, even though its not perfect.
>  @aaron.ballman wdyt?


The attached patch is nice and small: a 5-line generally useful utility function plus a few lines to call it.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D70144





More information about the cfe-commits mailing list