[PATCH] D69764: [clang-format] Add East/West Const fixer capability

Richard Smith - zygoloid via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 27 15:16:44 PDT 2020


rsmith added a comment.

In D69764#2058334 <https://reviews.llvm.org/D69764#2058334>, @MyDeveloperDay wrote:

> @rsmith, Thank you for your comments, I don't agree, but thank you anyway.
>
> I will continue to feel there is some value here, My hope is that others will feel the same.


To be clear: I do think a tool that will reorder specifiers to match a coding standard has value. My concern is that it seems like scope creep for clang-format in particular to do that, and that scope creep makes me uncomfortable -- we're moving further away from the set of transformations that clang-format can be very confident don't change the meaning of the program, and we have seen problems with such scope creep in the past. But I'm only uncomfortable, not opposed. Looking back through the review thread, I don't think I'm raising anything that @sammccall didn't already bring up, and it seems to me like you have consensus for doing this despite those concerns. So be it. :)

I think that if we are reordering `const`, we should be reordering all //decl-specifier//s -- I'd like to see `int static constexpr unsigned const long inline` reordered to something like `static constexpr inline const unsigned long int` too. Applying this only to `const` seems incomplete to me.



================
Comment at: clang/lib/Format/EastWestConstFixer.cpp:199
+  }
+  // Don't concern youself if nothing follows const.
+  if (!Tok->Next) {
----------------
(Typo: youself -> yourself)


================
Comment at: clang/lib/Format/EastWestConstFixer.cpp:291
+  if (isCVQualifierOrType(Tok) && isCVQualifierOrType(Tok->Next) && isCVQualifierOrType(Tok->Next->Next) &&
+    // `unsigned longl long const` -> `const unsigned long long`.
+      Tok->Next->Next->Next && Tok->Next->Next->Next->is(tok::kw_const)) {
----------------
(Typo: longl -> long)


================
Comment at: clang/lib/Format/EastWestConstFixer.cpp:293
+      Tok->Next->Next->Next && Tok->Next->Next->Next->is(tok::kw_const)) {
+    swapFourTokens(SourceMgr, Fixes, Tok, Tok->Next, Tok->Next, Tok->Next->Next->Next,
+                         /*West=*/true);
----------------
There can be more than four type-specifiers / cv-qualifiers in a row. Eg: `unsigned long long int volatile const` -> `const volatile unsigned long long int`.


================
Comment at: clang/lib/Format/EastWestConstFixer.h:1
+//===--- EastWwestConstFixer.h ----------------------------------*- C++ -*-===//
+//
----------------
(Typo in file name.)


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

https://reviews.llvm.org/D69764





More information about the cfe-commits mailing list