[PATCH] D68682: Clang-tidy fix removals removing all non-blank text from a line should remove the line

Dmitri Gribenko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Oct 11 06:40:34 PDT 2019


gribozavr added a comment.

> I guess here's the high-level question: should all removals that remove all non-blank text from a line also delete the line?

I see your point, but as https://llvm.org/PR43583 shows, we have a much larger problem: textual replacements don't compose. So, whatever we do, it will only be hacky workarounds for specific high-value cases.

About how exactly to do it. It would be preferred if checkers that already know that they are deleting a full line, just deleted the newline characters. However, for other cases, I feel like the place where this patch implements newline deletion is not ideal. It implements newline deletion while applying all fixes. Applying fixes is not the only client of fixit information. Probably it is one of the least-important ones, to be honest. I don't know who will run clang-tidy and just trust it to apply whatever fixes it wants. More likely, the user will want to see a preview of findings and fixes, and fixit application will be done by some other tool. Therefore, implementing newline deletion in the code that applies all fixes is not going to be helpful for most workflows.

Therefore, I think the most appropriate place to do this cleanup is `cleanupAroundReplacements`. What do you think?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D68682





More information about the cfe-commits mailing list