AlexanderHederstaf updated this revision to Diff 500697.
AlexanderHederstaf added a comment.
Rebase after change to pointer to member was merged.
Add case for typename used for nested dependent names.
Add assertions for template closers/openers.
Add commented-out tests for two cases where right to left
produces code that does not compile.
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
@@ -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 <typename Func> explicit Action(Action<Func> 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<T volatile>::Bar<Type const, 5> const volatile A::*;",
+ "volatile const Foo<volatile T>::Bar<const Type, 5> A::*;",
+ Style);
+
verifyFormat("int const Foo<int>::bat = 0;", "const int Foo<int>::bat = 0;",
Style);
verifyFormat("int const Foo<int>::bat = 0;", "int const Foo<int>::bat = 0;",
@@ -418,6 +427,15 @@
verifyFormat("unsigned long long const a;", "const unsigned long long a;",
Style);
+ // Multiple template parameters.
+ verifyFormat("Bar<std::Foo const, 32>", "Bar<const std::Foo, 32>", Style);
+ // Variable declaration based on template type.
+ verifyFormat("Bar<std::Foo const> bar", "Bar<const std::Foo> 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<int", Style);
verifyFormat("const std::Foo<int>", "const std::Foo<int>", Style);
+ // TODO: Fails to move const past std::Foo as it's not the last template
+ // parameter.
+ // Multiple template parameters.
+ // verifyFormat("Bar<const std::Foo, 32>", "Bar<std::Foo const, 32>", Style);
+
+ // TODO: Fails with "std::const Foo" when going left.
+ // Variable declaration based on template type.
+ // verifyFormat("Bar<const std::Foo> bar", "Bar<std::Foo const> 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 <typename T> Foo const f();",
+ "template <typename T> const Foo f();", Style);
+ verifyFormat("template <typename T> int const f();",
+ "template <typename T> const int 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
@@ -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 <class T> const Bar Foo()` ->
+ // `template <class T> Bar const Foo()`
+ // The cases:
+ // `Foo<int> const foo` -> `Foo<int> const foo`
+ // `Foo<int> volatile const` -> `Foo<int> 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<int>::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<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;
+ }
+
+ // 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<T> &&` -> `std::Foo<T> 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<int>::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;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
