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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to