[PATCH] D59859: [clang-tidy] FIXIT for implicit bool conversion now obeys upper case suffixes if enforced.

Alexander Kornienko via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 1 08:28:04 PDT 2019


alexfh added a comment.

In D59859#1444333 <https://reviews.llvm.org/D59859#1444333>, @aaron.ballman wrote:

> In D59859#1444176 <https://reviews.llvm.org/D59859#1444176>, @lebedev.ri wrote:
>
> > I'm not sure why we want this? What is wrong with simply applying clang-tidy twice?
>
>
> It doubles the execution time of checking a large project (which may be unacceptably slow), and there's no guarantee that twice will be enough in the general case (one set of fixes may trigger a different check's diagnostics, which may get fixed and trigger another check's diagnostics, etc).


Yeah, the situation when one automated fix generates another warning is not a nice user experience. It's not only about clang-tidy run time, it's also about annoying users and making their workflow less efficient.  Thus it's better to generate clang-tidy-clean code in fixes, where possible and not particularly difficult to implement.

As for the readability-uppercase-literal-suffix check (, I wonder whether clang-format could do this? Non-whitespace changes were historically not desired in clang-format, but there are a number of features there that generate non-whitespace changes (e.g. comment formatting, string literal splitting, #include sorting, macro formatting). This one seems to be local and quite safe. Maybe propose this feature to clang-format?

If the clang-format feature is viable, clang-tidy can already apply clang-format formatting. If not, I'm also not opposed to the new option for readability-implicit-bool-conversion (but maybe it should be a global option like `StrictMode`)?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59859





More information about the cfe-commits mailing list