diff --git a/clang/lib/Format/QualifierAlignmentFixer.cpp b/clang/lib/Format/QualifierAlignmentFixer.cpp --- a/clang/lib/Format/QualifierAlignmentFixer.cpp +++ b/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::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 a;` - // `const ArrayRef &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 a;` - // `const ::ArrayRef &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 &&` -> `std::Foo 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::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; diff --git a/clang/unittests/Format/QualifierFixerTest.cpp b/clang/unittests/Format/QualifierFixerTest.cpp --- a/clang/unittests/Format/QualifierFixerTest.cpp +++ b/clang/unittests/Format/QualifierFixerTest.cpp @@ -353,6 +353,10 @@ verifyFormat("static int const bat;", "static const int bat;", Style); verifyFormat("static int const bat;", "static int const bat;", Style); + verifyFormat("Foo::Bar const volatile A::*;", + "volatile const Foo::Bar A::*;", + Style); + verifyFormat("int const Foo::bat = 0;", "const int Foo::bat = 0;", Style); verifyFormat("int const Foo::bat = 0;", "int const Foo::bat = 0;", @@ -581,6 +585,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 +601,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 +616,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 +632,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 +646,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 +740,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 +975,12 @@ Style.QualifierAlignment = FormatStyle::QAS_Custom; Style.QualifierOrder = {"type", "const"}; + // TODO: Whitespace issue with this replacement + verifyFormat("template Foo const f();", + "template \n" + "const Foo f();", + Style); + // TODO: Doesn't handle requires ... verifyFormat("template \n" " requires Concept\n" "void f();",