Skip to content

Commit

Permalink
[clang-format] Handle common C++ non-keyword types as such (#83709)
Browse files Browse the repository at this point in the history
Fixes #83400.
  • Loading branch information
owenca authored Mar 9, 2024
1 parent e1405e4 commit 0baef3b
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 41 deletions.
18 changes: 16 additions & 2 deletions clang/lib/Format/FormatToken.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,22 @@ bool FormatToken::isSimpleTypeSpecifier() const {
}
}

bool FormatToken::isTypeOrIdentifier() const {
return isSimpleTypeSpecifier() || Tok.isOneOf(tok::kw_auto, tok::identifier);
// Sorted common C++ non-keyword types.
static SmallVector<StringRef> CppNonKeywordTypes = {
"clock_t", "int16_t", "int32_t", "int64_t", "int8_t",
"intptr_t", "ptrdiff_t", "size_t", "time_t", "uint16_t",
"uint32_t", "uint64_t", "uint8_t", "uintptr_t",
};

bool FormatToken::isTypeName(bool IsCpp) const {
return is(TT_TypeName) || isSimpleTypeSpecifier() ||
(IsCpp && is(tok::identifier) &&
std::binary_search(CppNonKeywordTypes.begin(),
CppNonKeywordTypes.end(), TokenText));
}

bool FormatToken::isTypeOrIdentifier(bool IsCpp) const {
return isTypeName(IsCpp) || isOneOf(tok::kw_auto, tok::identifier);
}

bool FormatToken::isBlockIndentedInitRBrace(const FormatStyle &Style) const {
Expand Down
4 changes: 3 additions & 1 deletion clang/lib/Format/FormatToken.h
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,9 @@ struct FormatToken {
/// Determine whether the token is a simple-type-specifier.
[[nodiscard]] bool isSimpleTypeSpecifier() const;

[[nodiscard]] bool isTypeOrIdentifier() const;
[[nodiscard]] bool isTypeName(bool IsCpp) const;

[[nodiscard]] bool isTypeOrIdentifier(bool IsCpp) const;

bool isObjCAccessSpecifier() const {
return is(tok::at) && Next &&
Expand Down
28 changes: 16 additions & 12 deletions clang/lib/Format/QualifierAlignmentFixer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -268,20 +268,24 @@ const FormatToken *LeftRightQualifierAlignmentFixer::analyzeRight(
if (isPossibleMacro(TypeToken))
return Tok;

const bool IsCpp = Style.isCpp();

// 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`
// The case `const long long volatile int` -> `long long int const volatile`
if (TypeToken->isSimpleTypeSpecifier()) {
if (TypeToken->isTypeName(IsCpp)) {
// The case `const decltype(foo)` -> `const decltype(foo)`
// The case `const typeof(foo)` -> `const typeof(foo)`
// The case `const _Atomic(foo)` -> `const _Atomic(foo)`
if (TypeToken->isOneOf(tok::kw_decltype, tok::kw_typeof, tok::kw__Atomic))
return Tok;

const FormatToken *LastSimpleTypeSpecifier = TypeToken;
while (isQualifierOrType(LastSimpleTypeSpecifier->getNextNonComment()))
while (isQualifierOrType(LastSimpleTypeSpecifier->getNextNonComment(),
IsCpp)) {
LastSimpleTypeSpecifier = LastSimpleTypeSpecifier->getNextNonComment();
}

rotateTokens(SourceMgr, Fixes, Tok, LastSimpleTypeSpecifier,
/*Left=*/false);
Expand All @@ -291,7 +295,7 @@ const FormatToken *LeftRightQualifierAlignmentFixer::analyzeRight(
// The case `unsigned short const` -> `unsigned short const`
// The case:
// `unsigned short volatile const` -> `unsigned short const volatile`
if (PreviousCheck && PreviousCheck->isSimpleTypeSpecifier()) {
if (PreviousCheck && PreviousCheck->isTypeName(IsCpp)) {
if (LastQual != Tok)
rotateTokens(SourceMgr, Fixes, Tok, LastQual, /*Left=*/false);
return Tok;
Expand Down Expand Up @@ -408,11 +412,11 @@ const FormatToken *LeftRightQualifierAlignmentFixer::analyzeLeft(
// The case `volatile long long const int` -> `const volatile long long int`
// 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->isSimpleTypeSpecifier()) {
if (const bool IsCpp = Style.isCpp(); TypeToken->isTypeName(IsCpp)) {
const FormatToken *LastSimpleTypeSpecifier = TypeToken;
while (isConfiguredQualifierOrType(
LastSimpleTypeSpecifier->getPreviousNonComment(),
ConfiguredQualifierTokens)) {
ConfiguredQualifierTokens, IsCpp)) {
LastSimpleTypeSpecifier =
LastSimpleTypeSpecifier->getPreviousNonComment();
}
Expand Down Expand Up @@ -610,16 +614,16 @@ void prepareLeftRightOrderingForQualifierAlignmentFixer(
}
}

bool LeftRightQualifierAlignmentFixer::isQualifierOrType(
const FormatToken *const Tok) {
return Tok && (Tok->isSimpleTypeSpecifier() || Tok->is(tok::kw_auto) ||
isQualifier(Tok));
bool LeftRightQualifierAlignmentFixer::isQualifierOrType(const FormatToken *Tok,
bool IsCpp) {
return Tok &&
(Tok->isTypeName(IsCpp) || Tok->is(tok::kw_auto) || isQualifier(Tok));
}

bool LeftRightQualifierAlignmentFixer::isConfiguredQualifierOrType(
const FormatToken *const Tok,
const std::vector<tok::TokenKind> &Qualifiers) {
return Tok && (Tok->isSimpleTypeSpecifier() || Tok->is(tok::kw_auto) ||
const FormatToken *Tok, const std::vector<tok::TokenKind> &Qualifiers,
bool IsCpp) {
return Tok && (Tok->isTypeName(IsCpp) || Tok->is(tok::kw_auto) ||
isConfiguredQualifier(Tok, Qualifiers));
}

Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Format/QualifierAlignmentFixer.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,11 @@ class LeftRightQualifierAlignmentFixer : public TokenAnalyzer {
tok::TokenKind QualifierType);

// Is the Token a simple or qualifier type
static bool isQualifierOrType(const FormatToken *Tok);
static bool isQualifierOrType(const FormatToken *Tok, bool IsCpp = true);
static bool
isConfiguredQualifierOrType(const FormatToken *Tok,
const std::vector<tok::TokenKind> &Qualifiers);
const std::vector<tok::TokenKind> &Qualifiers,
bool IsCpp = true);

// Is the Token likely a Macro
static bool isPossibleMacro(const FormatToken *Tok);
Expand Down
30 changes: 15 additions & 15 deletions clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ class AnnotatingParser {
(CurrentToken->is(tok::l_paren) && CurrentToken->Next &&
CurrentToken->Next->isOneOf(tok::star, tok::amp, tok::caret));
if ((CurrentToken->Previous->isOneOf(tok::kw_const, tok::kw_auto) ||
CurrentToken->Previous->isSimpleTypeSpecifier()) &&
CurrentToken->Previous->isTypeName(IsCpp)) &&
!(CurrentToken->is(tok::l_brace) ||
(CurrentToken->is(tok::l_paren) && !ProbablyFunctionTypeLParen))) {
Contexts.back().IsExpression = false;
Expand Down Expand Up @@ -2573,7 +2573,7 @@ class AnnotatingParser {
return true;

// MyClass a;
if (PreviousNotConst->isSimpleTypeSpecifier())
if (PreviousNotConst->isTypeName(IsCpp))
return true;

// type[] a in Java
Expand Down Expand Up @@ -2704,9 +2704,9 @@ class AnnotatingParser {
}

// Heuristically try to determine whether the parentheses contain a type.
auto IsQualifiedPointerOrReference = [](FormatToken *T) {
auto IsQualifiedPointerOrReference = [this](FormatToken *T) {
// This is used to handle cases such as x = (foo *const)&y;
assert(!T->isSimpleTypeSpecifier() && "Should have already been checked");
assert(!T->isTypeName(IsCpp) && "Should have already been checked");
// Strip trailing qualifiers such as const or volatile when checking
// whether the parens could be a cast to a pointer/reference type.
while (T) {
Expand Down Expand Up @@ -2738,7 +2738,7 @@ class AnnotatingParser {
bool ParensAreType =
!Tok.Previous ||
Tok.Previous->isOneOf(TT_TemplateCloser, TT_TypeDeclarationParen) ||
Tok.Previous->isSimpleTypeSpecifier() ||
Tok.Previous->isTypeName(IsCpp) ||
IsQualifiedPointerOrReference(Tok.Previous);
bool ParensCouldEndDecl =
Tok.Next->isOneOf(tok::equal, tok::semi, tok::l_brace, tok::greater);
Expand Down Expand Up @@ -3595,7 +3595,8 @@ static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current,
if (!Current.Tok.getIdentifierInfo())
return false;

auto skipOperatorName = [](const FormatToken *Next) -> const FormatToken * {
auto skipOperatorName =
[IsCpp](const FormatToken *Next) -> const FormatToken * {
for (; Next; Next = Next->Next) {
if (Next->is(TT_OverloadedOperatorLParen))
return Next;
Expand All @@ -3614,7 +3615,7 @@ static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current,
Next = Next->Next;
continue;
}
if ((Next->isSimpleTypeSpecifier() || Next->is(tok::identifier)) &&
if ((Next->isTypeName(IsCpp) || Next->is(tok::identifier)) &&
Next->Next && Next->Next->isPointerOrReference()) {
// For operator void*(), operator char*(), operator Foo*().
Next = Next->Next;
Expand Down Expand Up @@ -3712,9 +3713,8 @@ static bool isFunctionDeclarationName(bool IsCpp, const FormatToken &Current,
Tok = Tok->MatchingParen;
continue;
}
if (Tok->is(tok::kw_const) || Tok->isSimpleTypeSpecifier() ||
Tok->isOneOf(TT_PointerOrReference, TT_StartOfName, tok::ellipsis,
TT_TypeName)) {
if (Tok->is(tok::kw_const) || Tok->isTypeName(IsCpp) ||
Tok->isOneOf(TT_PointerOrReference, TT_StartOfName, tok::ellipsis)) {
return true;
}
if (Tok->isOneOf(tok::l_brace, TT_ObjCMethodExpr) || Tok->Tok.isLiteral())
Expand Down Expand Up @@ -4376,7 +4376,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
if (Left.Tok.isLiteral())
return true;
// for (auto a = 0, b = 0; const auto & c : {1, 2, 3})
if (Left.isTypeOrIdentifier() && Right.Next && Right.Next->Next &&
if (Left.isTypeOrIdentifier(IsCpp) && Right.Next && Right.Next->Next &&
Right.Next->Next->is(TT_RangeBasedForLoopColon)) {
return getTokenPointerOrReferenceAlignment(Right) !=
FormatStyle::PAS_Left;
Expand Down Expand Up @@ -4419,8 +4419,8 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
if (Right.is(tok::l_brace) && Right.is(BK_Block))
return true;
// for (auto a = 0, b = 0; const auto& c : {1, 2, 3})
if (Left.Previous && Left.Previous->isTypeOrIdentifier() && Right.Next &&
Right.Next->is(TT_RangeBasedForLoopColon)) {
if (Left.Previous && Left.Previous->isTypeOrIdentifier(IsCpp) &&
Right.Next && Right.Next->is(TT_RangeBasedForLoopColon)) {
return getTokenPointerOrReferenceAlignment(Left) !=
FormatStyle::PAS_Right;
}
Expand Down Expand Up @@ -4458,7 +4458,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
if (Right.isPointerOrReference()) {
const FormatToken *Previous = &Left;
while (Previous && Previous->isNot(tok::kw_operator)) {
if (Previous->is(tok::identifier) || Previous->isSimpleTypeSpecifier()) {
if (Previous->is(tok::identifier) || Previous->isTypeName(IsCpp)) {
Previous = Previous->getPreviousNonComment();
continue;
}
Expand Down Expand Up @@ -4647,7 +4647,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
if (!Style.isVerilog() &&
(Left.isOneOf(tok::identifier, tok::greater, tok::r_square,
tok::r_paren) ||
Left.isSimpleTypeSpecifier()) &&
Left.isTypeName(IsCpp)) &&
Right.is(tok::l_brace) && Right.getNextNonComment() &&
Right.isNot(BK_Block)) {
return false;
Expand Down
13 changes: 6 additions & 7 deletions clang/lib/Format/UnwrappedLineParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1865,8 +1865,7 @@ void UnwrappedLineParser::parseStructuralElement(
case tok::caret:
nextToken();
// Block return type.
if (FormatTok->Tok.isAnyIdentifier() ||
FormatTok->isSimpleTypeSpecifier()) {
if (FormatTok->Tok.isAnyIdentifier() || FormatTok->isTypeName(IsCpp)) {
nextToken();
// Return types: pointers are ok too.
while (FormatTok->is(tok::star))
Expand Down Expand Up @@ -2222,7 +2221,7 @@ bool UnwrappedLineParser::tryToParseLambda() {
bool InTemplateParameterList = false;

while (FormatTok->isNot(tok::l_brace)) {
if (FormatTok->isSimpleTypeSpecifier()) {
if (FormatTok->isTypeName(IsCpp)) {
nextToken();
continue;
}
Expand Down Expand Up @@ -3415,7 +3414,7 @@ bool clang::format::UnwrappedLineParser::parseRequires() {
break;
}
default:
if (PreviousNonComment->isTypeOrIdentifier()) {
if (PreviousNonComment->isTypeOrIdentifier(IsCpp)) {
// This is a requires clause.
parseRequiresClause(RequiresToken);
return true;
Expand Down Expand Up @@ -3478,7 +3477,7 @@ bool clang::format::UnwrappedLineParser::parseRequires() {
--OpenAngles;
break;
default:
if (NextToken->isSimpleTypeSpecifier()) {
if (NextToken->isTypeName(IsCpp)) {
FormatTok = Tokens->setPosition(StoredPosition);
parseRequiresExpression(RequiresToken);
return false;
Expand Down Expand Up @@ -3962,8 +3961,8 @@ void UnwrappedLineParser::parseRecord(bool ParseAsExpr) {
}
if (FormatTok->is(tok::l_square)) {
FormatToken *Previous = FormatTok->Previous;
if (!Previous ||
!(Previous->is(tok::r_paren) || Previous->isTypeOrIdentifier())) {
if (!Previous || (Previous->isNot(tok::r_paren) &&
!Previous->isTypeOrIdentifier(IsCpp))) {
// Don't try parsing a lambda if we had a closing parenthesis before,
// it was probably a pointer to an array: int (*)[].
if (!tryToParseLambda())
Expand Down
26 changes: 24 additions & 2 deletions clang/unittests/Format/TokenAnnotatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -620,6 +620,24 @@ TEST_F(TokenAnnotatorTest, UnderstandsCasts) {
ASSERT_EQ(Tokens.size(), 8u) << Tokens;
EXPECT_TOKEN(Tokens[3], tok::r_paren, TT_Unknown);
EXPECT_TOKEN(Tokens[4], tok::amp, TT_BinaryOperator);

Tokens = annotate("#define FOO(bar) foo((uint64_t)&bar)");
ASSERT_EQ(Tokens.size(), 15u) << Tokens;
EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_CastRParen);
EXPECT_TOKEN(Tokens[11], tok::amp, TT_UnaryOperator);

Tokens = annotate("#define FOO(bar) foo((Foo) & bar)");
ASSERT_EQ(Tokens.size(), 15u) << Tokens;
EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_Unknown);
EXPECT_TOKEN(Tokens[11], tok::amp, TT_BinaryOperator);

auto Style = getLLVMStyle();
Style.TypeNames.push_back("Foo");
Tokens = annotate("#define FOO(bar) foo((Foo)&bar)", Style);
ASSERT_EQ(Tokens.size(), 15u) << Tokens;
EXPECT_TOKEN(Tokens[9], tok::identifier, TT_TypeName);
EXPECT_TOKEN(Tokens[10], tok::r_paren, TT_CastRParen);
EXPECT_TOKEN(Tokens[11], tok::amp, TT_UnaryOperator);
}

TEST_F(TokenAnnotatorTest, UnderstandsDynamicExceptionSpecifier) {
Expand Down Expand Up @@ -1751,9 +1769,13 @@ TEST_F(TokenAnnotatorTest, UnderstandsFunctionDeclarationNames) {
EXPECT_TOKEN(Tokens[3], tok::identifier, TT_Unknown);
EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_FunctionTypeLParen);

Tokens = annotate("int iso_time(time_t);");
ASSERT_EQ(Tokens.size(), 7u) << Tokens;
EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);

auto Style = getLLVMStyle();
Style.TypeNames.push_back("time_t");
Tokens = annotate("int iso_time(time_t);", Style);
Style.TypeNames.push_back("MyType");
Tokens = annotate("int iso_time(MyType);", Style);
ASSERT_EQ(Tokens.size(), 7u) << Tokens;
EXPECT_TOKEN(Tokens[1], tok::identifier, TT_FunctionDeclarationName);
EXPECT_TOKEN(Tokens[3], tok::identifier, TT_TypeName);
Expand Down

0 comments on commit 0baef3b

Please sign in to comment.