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

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue May 26 07:00:41 PDT 2020


aaron.ballman added inline comments.


================
Comment at: clang/lib/Format/EastWestConstFixer.cpp:139
+  return (Tok->isSimpleTypeSpecifier() ||
+          Tok->isOneOf(tok::kw_volatile, tok::kw_auto));
+}
----------------
Do you need to look for `restrict` here as well as `volatile`?


================
Comment at: clang/lib/Format/EastWestConstFixer.cpp:150
+  }
+  // don't concern youself if nothing follows const
+  if (!Tok->Next) {
----------------
Comments should start with a capital letter and end with punctuation per the coding standard (same applies to other comments in the patch).


================
Comment at: clang/lib/Format/EastWestConstFixer.cpp:157-158
+      IsCVQualifierOrType(Tok->Next->Next)) {
+    // The unsigned/signed case  `const unsigned int` -> `unsigned int
+    // const`
+    swapFirstThreeTokens(SourceMgr, Fixes, Tok, Tok->Next, Tok->Next->Next,
----------------
What about something like `const unsigned long long` or the less-common-but-equally-valid `long const long unsigned`? Or multiple qualifiers like `unsigned volatile long const long * restrict`


================
Comment at: clang/lib/Format/Format.cpp:2547
 
+  if (Style.isCpp() || Style.Language == FormatStyle::LK_ObjC) {
+    if (Style.ConstStyle != FormatStyle::CS_Leave)
----------------
MyDeveloperDay wrote:
> aaron.ballman wrote:
> > This prevents us from using this in C code despite C having qualifiers that can go to the left or right of the base type but still allows you to use if from Objective-C. That seems incorrect.
> clang-format's isCpp() covers C and C++ (and ObjectiveC and ObjectiveC++)
> 
> but you did highlight that I don't need the extra LK_ObjC check
> clang-format's isCpp() covers C and C++ (and ObjectiveC and ObjectiveC++)

Wow, that's a really poorly named function then! Thank you for the clarification.


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

https://reviews.llvm.org/D69764





More information about the cfe-commits mailing list