AlexanderHederstaf updated this revision to Diff 500215.
AlexanderHederstaf added a comment.

Fixed the const/volatile placement. Added a few tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144709

Files:
  clang/lib/Format/QualifierAlignmentFixer.cpp
  clang/unittests/Format/QualifierFixerTest.cpp

Index: clang/unittests/Format/QualifierFixerTest.cpp
===================================================================
--- clang/unittests/Format/QualifierFixerTest.cpp
+++ clang/unittests/Format/QualifierFixerTest.cpp
@@ -581,6 +581,13 @@
   verifyFormat("const volatile int a;", "int volatile const a;", Style);
   verifyFormat("const volatile int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("const volatile Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("const volatile Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("const volatile Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "const", "volatile"};
 
@@ -590,6 +597,12 @@
   verifyFormat("int const volatile a;", "int volatile const a;", Style);
   verifyFormat("int const volatile a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo const volatile a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo const volatile a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo const volatile a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Left;
   Style.QualifierOrder = {"volatile", "const", "type"};
 
@@ -599,6 +612,13 @@
   verifyFormat("volatile const int a;", "int volatile const a;", Style);
   verifyFormat("volatile const int a;", "const int volatile a;", Style);
 
+  // TODO: Left style can not move qualifiers past Foo.
+  // verifyFormat("volatile const Foo a;", "const volatile Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "volatile const Foo a;", Style);
+  // verifyFormat("volatile const Foo a;", "Foo const volatile a;", Style);
+  // verifyFormat("volatile const Foo a;", "Foo volatile const a;", Style);
+  // verifyFormat("volatile const Foo a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Right;
   Style.QualifierOrder = {"type", "volatile", "const"};
 
@@ -608,6 +628,12 @@
   verifyFormat("int volatile const a;", "int volatile const a;", Style);
   verifyFormat("int volatile const a;", "const int volatile a;", Style);
 
+  verifyFormat("Foo volatile const a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo volatile const a;", "const Foo volatile a;", Style);
+
   Style.QualifierAlignment = FormatStyle::QAS_Custom;
   Style.QualifierOrder = {"type", "volatile", "const"};
 
@@ -616,6 +642,12 @@
   verifyFormat("int volatile const a;", "int const volatile a;", Style);
   verifyFormat("int volatile const a;", "int volatile const a;", Style);
   verifyFormat("int volatile const a;", "const int volatile a;", Style);
+
+  verifyFormat("Foo volatile const a;", "const volatile Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "volatile const Foo a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo const volatile a;", Style);
+  verifyFormat("Foo volatile const a;", "Foo volatile const a;", Style);
+  verifyFormat("Foo volatile const a;", "const Foo volatile a;", Style);
 }
 
 TEST_F(QualifierFixerTest, InlineStatics) {
@@ -704,6 +736,21 @@
 
   verifyFormat("static inline int const volatile *a const;",
                "const int inline static  volatile *a const;", Style);
+
+  verifyFormat("static inline Foo const volatile a;",
+               "const inline static volatile Foo a;", Style);
+  verifyFormat("static inline Foo const volatile a;",
+               "volatile inline static const Foo a;", Style);
+  // TODO: Fails as static or inline can't be moved past Foo to the left.
+  // verifyFormat("static inline Foo const volatile a;",
+  //              "Foo const inline static  volatile a;", Style);
+  // verifyFormat("static inline Foo const volatile a;",
+  //              "Foo volatile inline static  const a;", Style);
+  // verifyFormat("static inline Foo const volatile a;",
+  //              "const Foo inline static  volatile a;", Style);
+
+  // verifyFormat("static inline Foo const volatile *a const;",
+  //              "const Foo inline static  volatile *a const;", Style);
 }
 
 TEST_F(QualifierFixerTest, PrepareLeftRightOrdering) {
@@ -924,6 +971,12 @@
   Style.QualifierAlignment = FormatStyle::QAS_Custom;
   Style.QualifierOrder = {"type", "const"};
 
+  // TODO: Whitespace issue with this replacement
+  verifyFormat("template <typename T> Foo const f();",
+               "template <typename T>\n"
+               "const Foo f();",
+               Style);
+  // TODO: Doesn't handle requires ...
   verifyFormat("template <typename T>\n"
                "  requires Concept<T const>\n"
                "void f();",
Index: clang/lib/Format/QualifierAlignmentFixer.cpp
===================================================================
--- clang/lib/Format/QualifierAlignmentFixer.cpp
+++ clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -217,62 +217,70 @@
   if (LeftRightQualifierAlignmentFixer::isPossibleMacro(Tok->Next))
     return Tok;
 
-  auto AnalyzeTemplate =
-      [&](const FormatToken *Tok,
-          const FormatToken *StartTemplate) -> const FormatToken * {
-    // Read from the TemplateOpener to TemplateCloser.
-    FormatToken *EndTemplate = StartTemplate->MatchingParen;
-    if (EndTemplate) {
-      // Move to the end of any template class members e.g.
-      // `Foo<int>::iterator`.
-      if (EndTemplate->startsSequence(TT_TemplateCloser, tok::coloncolon,
-                                      tok::identifier)) {
-        EndTemplate = EndTemplate->Next->Next;
-      }
-    }
-    if (EndTemplate && EndTemplate->Next &&
-        !EndTemplate->Next->isOneOf(tok::equal, tok::l_paren)) {
-      insertQualifierAfter(SourceMgr, Fixes, EndTemplate, Qualifier);
-      // Remove the qualifier.
-      removeToken(SourceMgr, Fixes, Tok);
-      return Tok;
+  const FormatToken *PreviousCheck = Tok->Previous;
+  while (PreviousCheck && llvm::is_contained(ConfiguredQualifierTokens,
+                                             PreviousCheck->Tok.getKind())) {
+    PreviousCheck = PreviousCheck->Previous;
+  }
+
+  const bool IsEastQualifier = PreviousCheck && [PreviousCheck]() {
+    if (PreviousCheck->is(tok::r_paren)) {
+      return true;
+    } else if (PreviousCheck->is(TT_TemplateCloser)) {
+      return PreviousCheck->MatchingParen->Previous->isNot(tok::kw_template);
+    } else if (PreviousCheck->isOneOf(TT_PointerOrReference, tok::identifier,
+                                      tok::kw_auto)) {
+      return true;
     }
-    return nullptr;
-  };
+    return false;
+  }();
 
-  FormatToken *Qual = Tok->Next;
-  FormatToken *LastQual = Qual;
-  while (Qual && isQualifierOrType(Qual, ConfiguredQualifierTokens)) {
-    LastQual = Qual;
-    Qual = Qual->Next;
+  const FormatToken *LastQual = Tok;
+  while (LastQual->Next && llvm::is_contained(ConfiguredQualifierTokens,
+                                              LastQual->Next->Tok.getKind())) {
+    LastQual = LastQual->Next;
   }
-  if (LastQual && Qual != LastQual) {
-    rotateTokens(SourceMgr, Fixes, Tok, LastQual, /*Left=*/false);
-    Tok = LastQual;
-  } else if (Tok->startsSequence(QualifierType, tok::identifier,
-                                 TT_TemplateCloser)) {
-    FormatToken *Closer = Tok->Next->Next;
-    rotateTokens(SourceMgr, Fixes, Tok, Tok->Next, /*Left=*/false);
-    Tok = Closer;
+
+  if (IsEastQualifier) {
+    if (LastQual != Tok)
+      rotateTokens(SourceMgr, Fixes, Tok, LastQual, /*Left=*/false);
     return Tok;
-  } else if (Tok->startsSequence(QualifierType, tok::identifier,
-                                 TT_TemplateOpener)) {
-    // `const ArrayRef<int> a;`
-    // `const ArrayRef<int> &a;`
-    const FormatToken *NewTok = AnalyzeTemplate(Tok, Tok->Next->Next);
-    if (NewTok)
-      return NewTok;
-  } else if (Tok->startsSequence(QualifierType, tok::coloncolon,
-                                 tok::identifier, TT_TemplateOpener)) {
-    // `const ::ArrayRef<int> a;`
-    // `const ::ArrayRef<int> &a;`
-    const FormatToken *NewTok = AnalyzeTemplate(Tok, Tok->Next->Next->Next);
-    if (NewTok)
-      return NewTok;
-  } else if (Tok->startsSequence(QualifierType, tok::identifier) ||
-             Tok->startsSequence(QualifierType, tok::coloncolon,
-                                 tok::identifier)) {
-    FormatToken *Next = Tok->Next;
+  }
+
+  const FormatToken *TypeToken = LastQual->Next;
+  if (!TypeToken)
+    return Tok;
+
+  if (TypeToken->isSimpleTypeSpecifier()) {
+    const FormatToken *LastSimpleTypeSpecifier = TypeToken;
+    while (LastSimpleTypeSpecifier->Next &&
+           LastSimpleTypeSpecifier->Next->isSimpleTypeSpecifier()) {
+      LastSimpleTypeSpecifier = LastSimpleTypeSpecifier->Next;
+    }
+
+    // Place the Qualifier at the end of the list of qualifiers.
+    while (LastSimpleTypeSpecifier->Next &&
+           llvm::is_contained(ConfiguredQualifierTokens,
+                              LastSimpleTypeSpecifier->Next->Tok.getKind())) {
+      LastSimpleTypeSpecifier = LastSimpleTypeSpecifier->Next;
+    }
+
+    rotateTokens(SourceMgr, Fixes, Tok, LastSimpleTypeSpecifier,
+                 /*Left=*/false);
+    return LastSimpleTypeSpecifier;
+  } else if (PreviousCheck && PreviousCheck->isSimpleTypeSpecifier()) {
+    if (LastQual != Tok)
+      rotateTokens(SourceMgr, Fixes, Tok, LastQual, /*Left=*/false);
+    return Tok;
+  }
+
+  if (TypeToken->is(tok::coloncolon))
+    TypeToken = TypeToken->Next;
+
+  if (TypeToken->isOneOf(tok::kw_auto, tok::identifier)) {
+    const FormatToken *TypeEnd = TypeToken;
+
+    // The case  `const auto` -> `auto const`
     // The case  `const Foo` -> `Foo const`
     // The case  `const ::Foo` -> `::Foo const`
     // The case  `const Foo *` -> `Foo const *`
@@ -280,27 +288,23 @@
     // The case  `const Foo &&` -> `Foo const &&`
     // The case  `const std::Foo &&` -> `std::Foo const &&`
     // The case  `const std::Foo<T> &&` -> `std::Foo<T> const &&`
-    while (Next && Next->isOneOf(tok::identifier, tok::coloncolon))
-      Next = Next->Next;
-    if (Next && Next->is(TT_TemplateOpener)) {
-      Next = Next->MatchingParen;
-      // Move to the end of any template class members e.g.
-      // `Foo<int>::iterator`.
-      if (Next && Next->startsSequence(TT_TemplateCloser, tok::coloncolon,
-                                       tok::identifier)) {
-        return Tok;
-      }
-      assert(Next && "Missing template opener");
-      Next = Next->Next;
+    while (TypeEnd->Next &&
+           (TypeEnd->Next->startsSequence(tok::coloncolon, tok::identifier) ||
+            TypeEnd->Next->is(TT_TemplateOpener))) {
+      if (TypeEnd->Next->is(TT_TemplateOpener))
+        TypeEnd = TypeEnd->Next->MatchingParen;
+      else
+        TypeEnd = TypeEnd->Next->Next;
     }
-    if (Next && Next->isOneOf(tok::star, tok::amp, tok::ampamp) &&
-        !Tok->Next->isOneOf(Keywords.kw_override, Keywords.kw_final)) {
-      if (Next->Previous && !Next->Previous->is(QualifierType)) {
-        insertQualifierAfter(SourceMgr, Fixes, Next->Previous, Qualifier);
-        removeToken(SourceMgr, Fixes, Tok);
-      }
-      return Next;
+
+    // Place the Qualifier at the end of the list of qualifiers.
+    while (TypeEnd->Next && llvm::is_contained(ConfiguredQualifierTokens,
+                                               TypeEnd->Next->Tok.getKind())) {
+      TypeEnd = TypeEnd->Next;
     }
+
+    insertQualifierAfter(SourceMgr, Fixes, TypeEnd, Qualifier);
+    removeToken(SourceMgr, Fixes, Tok);
   }
 
   return Tok;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to