[PATCH] D131386: [clang-tidy] Added `ConstAlignment` option to `misc-const-correctness`

Mark de Wever via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Aug 15 09:12:14 PDT 2022


Mordante added a comment.

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

> We leave formatting decisions in clang-tidy to clang-format and I don't think we should deviate from that policy here without a very clear understanding of when we should relax that restriction. That said, I'm personally not certain we should have such an option (the long-term goal has generally been to integrate clang-format functionality into clang-tidy so there can be an option to just run format after applying fixes in a TU). Is there a compelling reason we should have it?

The reason for me to work on this feature is the fact that clang-format doesn't work reliable for changing the location of the `const`. After using clang-tidy and clang-format some `const`s were still one the right hand side. I didn't do a deep investigation, but it seems to affect classes in namespaces. So some occurrences of `std::string` were affected, but not all. So in the current state I can't use this clang-tidy fix.

Since clang-format doesn't fully parse the code I wonder whether all cases can be properly fixed. On the other hand clang-tidy has all information available.

Would integrating clang-format in clang-tidy mean clang-format will use the full information of the AST?



================
Comment at: clang-tools-extra/clang-tidy/misc/ConstCorrectnessCheck.cpp:66
+      ConstAlignment(
+          Options.get("ConstAlignment", utils::fixit::QualifierPolicy::Right)) {
   if (AnalyzeValues == false && AnalyzeReferences == false)
----------------
njames93 wrote:
> Mordante wrote:
> > I would suggest to use `QualifierAlignment` to match the name in clang-format.
> I thought about that, but the clang-format option doesn't just align the const qualifier. It works for all qualifiers.
> I am easy on what name we use for the option.
Fair point.


================
Comment at: clang-tools-extra/docs/clang-tidy/checks/misc/const-correctness.rst:149
     int *changing_pointee = &value;
     changing_pointee = &result;
 
----------------
Do you want to incorporate the D131859 here too or do you prefer that I make a separate review for that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131386



More information about the cfe-commits mailing list