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,115 @@ // 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 cases: + // `Foo() const` -> `Foo() const` + // `Foo() const final` -> `Foo() const final` + // `Foo() const override` -> `Foo() const final` + // `Foo() const volatile override` -> `Foo() const volatile override` + // `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 cases: + // `Foo const foo` -> `Foo const foo` + // `Foo volatile const` -> `Foo const volatile` + if (PreviousCheck->is(TT_TemplateCloser)) { + assert(TypeToken->Next->MatchingParen && "Missing template opener"); + assert(TypeToken->Next->MatchingParen->Previous && + "Missing identifier or keyword preceeding <>"); + 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; + } + + const FormatToken *TypeToken = LastQual->Next; + if (!TypeToken) + return Tok; + + if (isPossibleMacro(TypeToken)) 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; - } + // 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; + + // Skip the typename keyword + // The case `const typename C::type` -> `typename C::type const` + if (TypeToken->is(tok::kw_typename)) + 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,30 +330,29 @@ // The case `const Foo &&` -> `Foo const &&` // The case `const std::Foo &&` -> `std::Foo const &&` // The case `const std::Foo &&` -> `std::Foo const &&` - // However, `const Bar::*` remains the same. - while (Next && Next->isOneOf(tok::identifier, tok::coloncolon) && - !Next->startsSequence(tok::coloncolon, tok::star)) { - 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; + while (TypeToken->Next && + (TypeToken->Next->startsSequence(tok::coloncolon, tok::identifier) || + TypeToken->Next->is(TT_TemplateOpener))) { + if (TypeToken->Next->is(TT_TemplateOpener)) { + assert(TypeToken->Next->MatchingParen && "Missing template closer"); + TypeToken = TypeToken->Next->MatchingParen; + } else { + TypeToken = TypeToken->Next->Next; } - assert(Next && "Missing template opener"); - Next = 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;", @@ -418,6 +427,15 @@ verifyFormat("unsigned long long const a;", "const unsigned long long a;", Style); + // Multiple template parameters. + verifyFormat("Bar", "Bar", Style); + // Variable declaration based on template type. + verifyFormat("Bar bar", "Bar bar", Style); + + // Using typename for a nested dependent type name + verifyFormat("typename Foo::iterator const;", "const typename Foo::iterator;", + Style); + // don't adjust macros verifyFormat("const INTPTR a;", "const INTPTR a;", Style); @@ -573,6 +591,20 @@ verifyFormat("const std::Foo < int", "const std::Foo", "const std::Foo", Style); + // TODO: Fails to move const past std::Foo as it's not the last template + // parameter. + // Multiple template parameters. + // verifyFormat("Bar", "Bar", Style); + + // TODO: Fails with "std::const Foo" when going left. + // Variable declaration based on template type. + // verifyFormat("Bar bar", "Bar bar", Style); + + // TODO: Fails to move const past typename "typename const Foo::iterator". + // Using typename for a dependent name + // verifyFormat("const typename Foo::iterator", "typename Foo::iterator + // const", Style); + // don't adjust macros verifyFormat("INTPTR const a;", "INTPTR const a;", Style); @@ -601,6 +633,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"}; @@ -610,6 +649,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"}; @@ -619,6 +664,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"}; @@ -628,6 +680,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"}; @@ -636,6 +694,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) { @@ -724,6 +788,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) { @@ -944,6 +1023,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();",