[clang-format] Hanlde qualified type name for QualifierAlignment

[owenca]

Fixes #125178.

[llvmbot]

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Fixes #125178.

---

Full diff: https://github.com/llvm/llvm-project/pull/125327.diff

2 Files Affected:

• (modified) clang/lib/Format/QualifierAlignmentFixer.cpp (+7-1)
• (modified) clang/unittests/Format/QualifierFixerTest.cpp (+12)

diff --git a/clang/lib/Format/QualifierAlignmentFixer.cpp b/clang/lib/Format/QualifierAlignmentFixer.cpp
index 21fb5074b4928f..f777ac2a89f755 100644
--- a/clang/lib/Format/QualifierAlignmentFixer.cpp
+++ b/clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -132,8 +132,10 @@ static void rotateTokens(const SourceManager &SourceMgr,
   // Then move through the other tokens.
   auto *Tok = Begin;
   while (Tok != End) {
-    if (!NewText.empty() && !endsWithSpace(NewText))
+    if (!NewText.empty() && !endsWithSpace(NewText) &&
+        Tok->isNot(tok::coloncolon)) {
       NewText += " ";
+    }
 
     NewText += Tok->TokenText;
     Tok = Tok->Next;
@@ -412,6 +414,10 @@ const FormatToken *LeftRightQualifierAlignmentFixer::analyzeLeft(
   // The case `const long long volatile int` -> `const volatile long long int`
   // The case `long volatile long int const` -> `const volatile long long int`
   if (TypeToken->isTypeName(LangOpts)) {
+    if (const auto *Prev = TypeToken->getPreviousNonComment();
+        Prev && Prev->endsSequence(tok::coloncolon, tok::identifier)) {
+      TypeToken = Prev->getPreviousNonComment();
+    }
     const FormatToken *LastSimpleTypeSpecifier = TypeToken;
     while (isConfiguredQualifierOrType(
         LastSimpleTypeSpecifier->getPreviousNonComment(),
diff --git a/clang/unittests/Format/QualifierFixerTest.cpp b/clang/unittests/Format/QualifierFixerTest.cpp
index 129828b0d187a9..530587bbf2ca74 100644
--- a/clang/unittests/Format/QualifierFixerTest.cpp
+++ b/clang/unittests/Format/QualifierFixerTest.cpp
@@ -1291,6 +1291,18 @@ TEST_F(QualifierFixerTest, WithCpp11Attribute) {
                "[[maybe_unused]] constexpr static int A", Style);
 }
 
+TEST_F(QualifierFixerTest, WithQualifiedTypeName) {
+  auto Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"constexpr", "type", "const"};
+
+  verifyFormat("constexpr std::int64_t x{123};",
+               "std::int64_t constexpr x{123};", Style);
+
+  Style.TypeNames.push_back("bar");
+  verifyFormat("constexpr foo::bar x{12};", "foo::bar constexpr x{12};", Style);
+}
+
 TEST_F(QualifierFixerTest, DisableRegions) {
   FormatStyle Style = getLLVMStyle();
   Style.QualifierAlignment = FormatStyle::QAS_Custom;

[rymiel]

Seems to also work for top-level types (::int_64_t constexpr x{123}; works correctly) but breaks for fully qualified types (::std::int64_t constexpr x{123}; becomes ::constexpr std::int64_t x{123};)

[owenca]

Seems to also work for top-level types (::int_64_t constexpr x{123}; works correctly) but breaks for fully qualified types (::std::int64_t constexpr x{123}; becomes ::constexpr std::int64_t x{123};)

Yeah, I intentionally didn't want to use a loop for names like A1::A2:: ... ::An and instead just wanted to handle single-level nested names like A::B.

[HazardyKnusperkeks]

Seems to also work for top-level types (::int_64_t constexpr x{123}; works correctly) but breaks for fully qualified types (::std::int64_t constexpr x{123}; becomes ::constexpr std::int64_t x{123};)

Yeah, I intentionally didn't want to use a loop for names like A1::A2:: ... ::An and instead just wanted to handle single-level nested names like A::B.

But why?

[owenca]

Same reason we almost never handle them, just like we (almost?) never handle the possibility of templated qualified names of arbitrary nesting levels (e.g. A1<B1>::A2<B2> … An<Bn>) and of comments before every token. We can always tweak the code if and when we need to handle these rare cases.

[owenca]

Seems to also work for top-level types (::int_64_t constexpr x{123}; works correctly)

It didn't work either.

[owenca]

Seems to also work for top-level types (::int_64_t constexpr x{123}; works correctly) but breaks for fully qualified types (::std::int64_t constexpr x{123}; becomes ::constexpr std::int64_t x{123};)

Yeah, I intentionally didn't want to use a loop for names like A1::A2:: ... ::An and instead just wanted to handle single-level nested names like A::B.

Now it handles the general pattern above (and also ::A1::A2:: ... An).

[HazardyKnusperkeks]

Suggested change

	if (!(Prev && Prev->is(tok::identifier)))
	if (!Prev || Prev->isNot(tok::identifier))

I personally don't like negated parenthesis.

[owenca]

Neither do I when the conditions inside the parentheses are unrelated. Otherwise, I prefer the negated parentheses. Another example is !(Foo == A || Foo == B || Foo == C) (i.e. Foo is not one of A, B, and C) vs Foo != A && Foo != B && Foo != C (i.e. Foo is not A, not B, and not C). The former is more readable to me.

[mydeveloperday]

Thank you

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[owenca]

/cherry-pick eb6ca12

[llvmbot]

/pull-request #126839