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 @@ -206,7 +206,7 @@ const FormatToken *LeftRightQualifierAlignmentFixer::analyzeRight( const SourceManager &SourceMgr, const AdditionalKeywords &Keywords, - tooling::Replacements &Fixes, const FormatToken *Tok, + tooling::Replacements &Fixes, const FormatToken *const Tok, const std::string &Qualifier, tok::TokenKind QualifierType) { // We only need to think about streams that begin with a qualifier. if (!Tok->is(QualifierType)) @@ -214,65 +214,103 @@ // Don't concern yourself if nothing follows the qualifier. if (!Tok->Next) return Tok; - if (LeftRightQualifierAlignmentFixer::isPossibleMacro(Tok->Next)) + + // Skip by qualifiers to the left to find what preceeds the qualifiers. + const FormatToken *PreviousCheck = Tok->Previous; + while (PreviousCheck && llvm::is_contained(ConfiguredQualifierTokens, + PreviousCheck->Tok.getKind())) { + PreviousCheck = PreviousCheck->Previous; + } + + // Examples given in order of ['type', 'const', 'volatile'] + const bool IsRightQualifier = PreviousCheck && [PreviousCheck]() { + // The case `Foo() const` -> `Foo() const` + // The case `Foo() const final` -> `Foo() const final` + // The case `Foo() const override` -> `Foo() const final` + // The case `Foo() const volatile override` -> `Foo() const volatile + // override` The case `Foo() volatile const final` -> `Foo() const volatile + // final` + if (PreviousCheck->is(tok::r_paren)) + return true; + + // The case `template const Bar Foo()` -> `template Bar + // const Foo()` The case `Foo const foo` -> `Foo const foo` The + // case `Foo volatile const` -> `Foo const volatile` + if (PreviousCheck->is(TT_TemplateCloser)) + return PreviousCheck->MatchingParen->Previous->isNot(tok::kw_template); + + // The case `Foo* const` -> `Foo* const` + // The case `Foo* volatile const` -> `Foo* const volatile` + // The case `int32_t const` -> `int32_t const` + // The case `auto volatile const` -> `auto const volatile` + if (PreviousCheck->isOneOf(TT_PointerOrReference, tok::identifier, + tok::kw_auto)) { + return true; + } + + return false; + }(); + + // Find the last qualifier to the right. + const FormatToken *LastQual = Tok; + while (LastQual->Next && llvm::is_contained(ConfiguredQualifierTokens, + LastQual->Next->Tok.getKind())) { + LastQual = LastQual->Next; + } + + // If this qualifier is to the right of a type or pointer do a partial sort + // and return. + if (IsRightQualifier) { + if (LastQual != Tok) + rotateTokens(SourceMgr, Fixes, Tok, LastQual, /*Left=*/false); 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; - } + const FormatToken *TypeToken = LastQual->Next; + if (!TypeToken) + return Tok; + + if (isPossibleMacro(TypeToken)) + return Tok; + + // The case `const long long int volatile` -> `long long int const volatile` + // The case `long const long int volatile` -> `long long int const volatile` + // The case `long long volatile int const` -> `long long int const volatile` + if (TypeToken->isSimpleTypeSpecifier()) { + const FormatToken *LastSimpleTypeSpecifier = TypeToken; + while (LastSimpleTypeSpecifier->Next && + LastSimpleTypeSpecifier->Next->isSimpleTypeSpecifier()) { + LastSimpleTypeSpecifier = LastSimpleTypeSpecifier->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; + + // 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; } - return nullptr; - }; - FormatToken *Qual = Tok->Next; - FormatToken *LastQual = Qual; - while (Qual && isQualifierOrType(Qual, ConfiguredQualifierTokens)) { - LastQual = Qual; - Qual = Qual->Next; + rotateTokens(SourceMgr, Fixes, Tok, LastSimpleTypeSpecifier, + /*Left=*/false); + return LastSimpleTypeSpecifier; } - 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; + + // The case `unsigned short const` -> `unsigned short const` + // The case `unsigned short volatile const` -> `unsigned short const + // volatile` + if (PreviousCheck && PreviousCheck->isSimpleTypeSpecifier()) { + 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; + } + + // Skip the initial :: of a global-namespace type. + // The case `const ::...` -> `::... const` + if (TypeToken->is(tok::coloncolon)) + TypeToken = TypeToken->Next; + + if (TypeToken->isOneOf(tok::kw_auto, tok::identifier)) { + // 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 +318,27 @@ // 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 (TypeToken->Next && + (TypeToken->Next->startsSequence(tok::coloncolon, tok::identifier) || + TypeToken->Next->is(TT_TemplateOpener))) { + if (TypeToken->Next->is(TT_TemplateOpener)) + TypeToken = TypeToken->Next->MatchingParen; + else + TypeToken = TypeToken->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 (TypeToken->Next && + llvm::is_contained(ConfiguredQualifierTokens, + TypeToken->Next->Tok.getKind())) { + TypeToken = TypeToken->Next; } + + insertQualifierAfter(SourceMgr, Fixes, TypeToken, Qualifier); + // Remove token and following whitespace. + auto Range = CharSourceRange::getCharRange( + Tok->getStartOfNonWhitespace(), Tok->Next->getStartOfNonWhitespace()); + replaceToken(SourceMgr, Fixes, Range, ""); } 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 @@ -303,6 +303,11 @@ verifyFormat("void foo() const final;", Style); verifyFormat("void foo() const final LLVM_READONLY;", Style); verifyFormat("void foo() const LLVM_READONLY;", Style); + verifyFormat("void foo() const volatile override;", Style); + verifyFormat("void foo() const volatile override LLVM_READONLY;", Style); + verifyFormat("void foo() const volatile final;", Style); + verifyFormat("void foo() const volatile final LLVM_READONLY;", Style); + verifyFormat("void foo() const volatile LLVM_READONLY;", Style); verifyFormat( "template explicit Action(Action const &action);", @@ -353,6 +358,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 +590,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 +606,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 +621,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 +637,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 +651,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 +745,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 +980,11 @@ Style.QualifierAlignment = FormatStyle::QAS_Custom; Style.QualifierOrder = {"type", "const"}; + verifyFormat("template Foo const f();", + "template const Foo f();", Style); + verifyFormat("template int const f();", + "template const int f();", Style); + // TODO: Doesn't handle requires ... verifyFormat("template \n" " requires Concept\n" "void f();",