https://github.com/owenca updated https://github.com/llvm/llvm-project/pull/131434
>From 46cde1b7667f36115d746326b78d011511cac738 Mon Sep 17 00:00:00 2001 From: Owen Pan <owenpi...@gmail.com> Date: Sat, 15 Mar 2025 00:37:53 -0700 Subject: [PATCH 1/2] [clang-format] Correctly annotate user-defined conversion functions Also fix/delete existing invalid/redundant test cases. Fix #130894 --- clang/lib/Format/TokenAnnotator.cpp | 27 ++++++++-- clang/unittests/Format/FormatTest.cpp | 28 +++++----- clang/unittests/Format/TokenAnnotatorTest.cpp | 53 +++++++++++++++++++ 3 files changed, 89 insertions(+), 19 deletions(-) diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 08539de405c67..d618eab1692b3 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -1639,6 +1639,25 @@ class AnnotatingParser { case tok::kw_operator: if (Style.isProto()) break; + // C++ user-defined conversion function. + if (IsCpp && CurrentToken && + (CurrentToken->is(tok::kw_auto) || + CurrentToken->isTypeName(LangOpts))) { + FormatToken *LParen; + if (CurrentToken->startsSequence(tok::kw_decltype, tok::l_paren, + tok::kw_auto, tok::r_paren)) { + LParen = CurrentToken->Next->Next->Next->Next; + } else { + for (LParen = CurrentToken->Next; + LParen && LParen->isNot(tok::l_paren); LParen = LParen->Next) { + } + } + if (LParen && LParen->startsSequence(tok::l_paren, tok::r_paren)) { + Tok->setFinalizedType(TT_FunctionDeclarationName); + LParen->setFinalizedType(TT_FunctionDeclarationLParen); + break; + } + } while (CurrentToken && !CurrentToken->isOneOf(tok::l_paren, tok::semi, tok::r_paren)) { if (CurrentToken->isOneOf(tok::star, tok::amp)) @@ -3071,12 +3090,10 @@ class AnnotatingParser { if (InTemplateArgument && NextToken->Tok.isAnyIdentifier()) return TT_BinaryOperator; - // "&&" followed by "(", "*", or "&" is quite unlikely to be two successive - // unary "&". - if (Tok.is(tok::ampamp) && - NextToken->isOneOf(tok::l_paren, tok::star, tok::amp)) { + // "&&" followed by "*" or "&" is quite unlikely to be two successive unary + // "&". + if (Tok.is(tok::ampamp) && NextToken->isOneOf(tok::star, tok::amp)) return TT_BinaryOperator; - } // This catches some cases where evaluation order is used as control flow: // aaa && aaa->f(); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 9864e7ec1b2ec..5df7865f5a629 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -10443,27 +10443,17 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { "void\n" "A::operator->() {}\n" "void\n" - "A::operator void *() {}\n" + "A::operator&() {}\n" "void\n" - "A::operator void &() {}\n" - "void\n" - "A::operator void &&() {}\n" - "void\n" - "A::operator char *() {}\n" + "A::operator&&() {}\n" "void\n" "A::operator[]() {}\n" "void\n" "A::operator!() {}\n" "void\n" - "A::operator**() {}\n" - "void\n" "A::operator<Foo> *() {}\n" "void\n" - "A::operator<Foo> **() {}\n" - "void\n" - "A::operator<Foo> &() {}\n" - "void\n" - "A::operator void **() {}", + "A::operator<Foo> &() {}\n", Style); verifyFormat("constexpr auto\n" "operator()() const -> reference {}\n" @@ -10486,7 +10476,7 @@ TEST_F(FormatTest, ReturnTypeBreakingStyle) { "constexpr auto\n" "operator void &() const -> reference {}\n" "constexpr auto\n" - "operator void &&() const -> reference {}\n" + "operator&&() const -> reference {}\n" "constexpr auto\n" "operator char *() const -> reference {}\n" "constexpr auto\n" @@ -28032,6 +28022,16 @@ TEST_F(FormatTest, BreakAfterAttributes) { " --d;", CtrlStmtCode, Style); + verifyFormat("[[nodiscard]]\n" + "operator bool();\n" + "[[nodiscard]]\n" + "operator bool() {\n" + " return true;\n" + "}", + "[[nodiscard]] operator bool();\n" + "[[nodiscard]] operator bool() { return true; }", + Style); + constexpr StringRef CtorDtorCode("struct Foo {\n" " [[deprecated]] Foo();\n" " [[deprecated]] Foo() {}\n" diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp index 5e2d301c5d1f3..48927813af317 100644 --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -3856,6 +3856,59 @@ TEST_F(TokenAnnotatorTest, AfterPPDirective) { EXPECT_TOKEN(Tokens[2], tok::minusminus, TT_AfterPPDirective); } +TEST_F(TokenAnnotatorTest, UserDefinedConversionFunction) { + auto Tokens = annotate("operator int();"); + ASSERT_EQ(Tokens.size(), 6u) << Tokens; + EXPECT_TOKEN(Tokens[0], tok::kw_operator, TT_FunctionDeclarationName); + EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_FunctionDeclarationLParen); + + Tokens = annotate("explicit operator int *();"); + ASSERT_EQ(Tokens.size(), 8u) << Tokens; + EXPECT_TOKEN(Tokens[1], tok::kw_operator, TT_FunctionDeclarationName); + EXPECT_TOKEN(Tokens[3], tok::star, TT_PointerOrReference); + EXPECT_TOKEN(Tokens[4], tok::l_paren, TT_FunctionDeclarationLParen); + + Tokens = annotate("operator int &();"); + ASSERT_EQ(Tokens.size(), 7u) << Tokens; + EXPECT_TOKEN(Tokens[0], tok::kw_operator, TT_FunctionDeclarationName); + EXPECT_TOKEN(Tokens[2], tok::amp, TT_PointerOrReference); + EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_FunctionDeclarationLParen); + + Tokens = annotate("operator auto() const { return 2; }"); + ASSERT_EQ(Tokens.size(), 11u) << Tokens; + EXPECT_TOKEN(Tokens[0], tok::kw_operator, TT_FunctionDeclarationName); + EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_FunctionDeclarationLParen); + EXPECT_TOKEN(Tokens[4], tok::kw_const, TT_TrailingAnnotation); + EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace); + + Tokens = annotate("operator decltype(auto)() const;"); + ASSERT_EQ(Tokens.size(), 10u) << Tokens; + EXPECT_TOKEN(Tokens[0], tok::kw_operator, TT_FunctionDeclarationName); + EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_TypeDeclarationParen); + EXPECT_TOKEN(Tokens[4], tok::r_paren, TT_TypeDeclarationParen); + EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_FunctionDeclarationLParen); + EXPECT_TOKEN(Tokens[7], tok::kw_const, TT_TrailingAnnotation); + + auto Style = getLLVMStyle(); + Style.TypeNames.push_back("Foo"); + + Tokens = annotate("virtual operator Foo() = 0;", Style); + ASSERT_EQ(Tokens.size(), 9u) << Tokens; + EXPECT_TOKEN(Tokens[1], tok::kw_operator, TT_FunctionDeclarationName); + EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_FunctionDeclarationLParen); + + Tokens = annotate("operator Foo() override { return Foo(); }", Style); + ASSERT_EQ(Tokens.size(), 13u) << Tokens; + EXPECT_TOKEN(Tokens[0], tok::kw_operator, TT_FunctionDeclarationName); + EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_FunctionDeclarationLParen); + EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace); + + Tokens = annotate("friend Bar::operator Foo();", Style); + ASSERT_EQ(Tokens.size(), 9u) << Tokens; + EXPECT_TOKEN(Tokens[3], tok::kw_operator, TT_FunctionDeclarationName); + EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_FunctionDeclarationLParen); +} + } // namespace } // namespace format } // namespace clang >From 83b12702b5a7ddb0e0e5febc9b57c6361c28df27 Mon Sep 17 00:00:00 2001 From: Owen Pan <owenpi...@gmail.com> Date: Sun, 16 Mar 2025 00:09:28 -0700 Subject: [PATCH 2/2] Address review comments --- clang/lib/Format/FormatToken.h | 4 ++ clang/lib/Format/FormatTokenLexer.cpp | 6 +-- clang/lib/Format/TokenAnnotator.cpp | 46 +++++++++++-------- clang/unittests/Format/TokenAnnotatorTest.cpp | 13 ++---- 4 files changed, 38 insertions(+), 31 deletions(-) diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h index 77935e75d4b4c..3808872d227a9 100644 --- a/clang/lib/Format/FormatToken.h +++ b/clang/lib/Format/FormatToken.h @@ -746,6 +746,10 @@ struct FormatToken { return isOneOf(tok::star, tok::amp, tok::ampamp); } + bool isPlacementOperator() const { + return isOneOf(tok::kw_new, tok::kw_delete); + } + bool isUnaryOperator() const { switch (Tok.getKind()) { case tok::plus: diff --git a/clang/lib/Format/FormatTokenLexer.cpp b/clang/lib/Format/FormatTokenLexer.cpp index 16f0a76f3a954..eed54a11684b5 100644 --- a/clang/lib/Format/FormatTokenLexer.cpp +++ b/clang/lib/Format/FormatTokenLexer.cpp @@ -610,9 +610,9 @@ bool FormatTokenLexer::precedesOperand(FormatToken *Tok) { tok::r_brace, tok::l_square, tok::semi, tok::exclaim, tok::colon, tok::question, tok::tilde) || Tok->isOneOf(tok::kw_return, tok::kw_do, tok::kw_case, tok::kw_throw, - tok::kw_else, tok::kw_new, tok::kw_delete, tok::kw_void, - tok::kw_typeof, Keywords.kw_instanceof, Keywords.kw_in) || - Tok->isBinaryOperator(); + tok::kw_else, tok::kw_void, tok::kw_typeof, + Keywords.kw_instanceof, Keywords.kw_in) || + Tok->isPlacementOperator() || Tok->isBinaryOperator(); } bool FormatTokenLexer::canPrecedeRegexLiteral(FormatToken *Prev) { diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index d618eab1692b3..abce6b04f5070 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -1639,23 +1639,29 @@ class AnnotatingParser { case tok::kw_operator: if (Style.isProto()) break; - // C++ user-defined conversion function. - if (IsCpp && CurrentToken && - (CurrentToken->is(tok::kw_auto) || - CurrentToken->isTypeName(LangOpts))) { - FormatToken *LParen; - if (CurrentToken->startsSequence(tok::kw_decltype, tok::l_paren, - tok::kw_auto, tok::r_paren)) { - LParen = CurrentToken->Next->Next->Next->Next; - } else { - for (LParen = CurrentToken->Next; - LParen && LParen->isNot(tok::l_paren); LParen = LParen->Next) { + // Handle C++ user-defined conversion function. + if (IsCpp && CurrentToken) { + const auto *Info = CurrentToken->Tok.getIdentifierInfo(); + // What follows Tok is an identifier or a non-operator keyword. + if (Info && !(Info->isCPlusPlusOperatorKeyword() || + CurrentToken->isPlacementOperator() || + CurrentToken->is(tok::kw_co_await))) { + FormatToken *LParen; + if (CurrentToken->startsSequence(tok::kw_decltype, tok::l_paren, + tok::kw_auto, tok::r_paren)) { + // Skip `decltype(auto)`. + LParen = CurrentToken->Next->Next->Next->Next; + } else { + // Skip to l_paren. + for (LParen = CurrentToken->Next; + LParen && LParen->isNot(tok::l_paren); LParen = LParen->Next) { + } + } + if (LParen && LParen->is(tok::l_paren)) { + Tok->setFinalizedType(TT_FunctionDeclarationName); + LParen->setFinalizedType(TT_FunctionDeclarationLParen); + break; } - } - if (LParen && LParen->startsSequence(tok::l_paren, tok::r_paren)) { - Tok->setFinalizedType(TT_FunctionDeclarationName); - LParen->setFinalizedType(TT_FunctionDeclarationLParen); - break; } } while (CurrentToken && @@ -3018,7 +3024,7 @@ class AnnotatingParser { return TT_UnaryOperator; if (PrevToken->is(TT_TypeName)) return TT_PointerOrReference; - if (PrevToken->isOneOf(tok::kw_new, tok::kw_delete) && Tok.is(tok::ampamp)) + if (PrevToken->isPlacementOperator() && Tok.is(tok::ampamp)) return TT_BinaryOperator; const FormatToken *NextToken = Tok.getNextNonComment(); @@ -3808,7 +3814,7 @@ static bool isFunctionDeclarationName(const LangOptions &LangOpts, return Next; if (Next->is(TT_OverloadedOperator)) continue; - if (Next->isOneOf(tok::kw_new, tok::kw_delete, tok::kw_co_await)) { + if (Next->isPlacementOperator() || Next->is(tok::kw_co_await)) { // For 'new[]' and 'delete[]'. if (Next->Next && Next->Next->startsSequence(tok::l_square, tok::r_square)) { @@ -4819,7 +4825,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line, spaceRequiredBeforeParens(Right); } if (Style.SpaceBeforeParens == FormatStyle::SBPO_Custom && - Left.isOneOf(tok::kw_new, tok::kw_delete) && + Left.isPlacementOperator() && Right.isNot(TT_OverloadedOperatorLParen) && !(Line.MightBeFunctionDecl && Left.is(TT_FunctionDeclarationName))) { const auto *RParen = Right.MatchingParen; @@ -4862,7 +4868,7 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line, return Style.SpaceBeforeParensOptions.AfterControlStatements || spaceRequiredBeforeParens(Right); } - if (Left.isOneOf(tok::kw_new, tok::kw_delete) || + if (Left.isPlacementOperator() || (Left.is(tok::r_square) && Left.MatchingParen && Left.MatchingParen->Previous && Left.MatchingParen->Previous->is(tok::kw_delete))) { diff --git a/clang/unittests/Format/TokenAnnotatorTest.cpp b/clang/unittests/Format/TokenAnnotatorTest.cpp index 48927813af317..1ff785110fc34 100644 --- a/clang/unittests/Format/TokenAnnotatorTest.cpp +++ b/clang/unittests/Format/TokenAnnotatorTest.cpp @@ -3857,8 +3857,8 @@ TEST_F(TokenAnnotatorTest, AfterPPDirective) { } TEST_F(TokenAnnotatorTest, UserDefinedConversionFunction) { - auto Tokens = annotate("operator int();"); - ASSERT_EQ(Tokens.size(), 6u) << Tokens; + auto Tokens = annotate("operator int(void);"); + ASSERT_EQ(Tokens.size(), 7u) << Tokens; EXPECT_TOKEN(Tokens[0], tok::kw_operator, TT_FunctionDeclarationName); EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_FunctionDeclarationLParen); @@ -3889,21 +3889,18 @@ TEST_F(TokenAnnotatorTest, UserDefinedConversionFunction) { EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_FunctionDeclarationLParen); EXPECT_TOKEN(Tokens[7], tok::kw_const, TT_TrailingAnnotation); - auto Style = getLLVMStyle(); - Style.TypeNames.push_back("Foo"); - - Tokens = annotate("virtual operator Foo() = 0;", Style); + Tokens = annotate("virtual operator Foo() = 0;"); ASSERT_EQ(Tokens.size(), 9u) << Tokens; EXPECT_TOKEN(Tokens[1], tok::kw_operator, TT_FunctionDeclarationName); EXPECT_TOKEN(Tokens[3], tok::l_paren, TT_FunctionDeclarationLParen); - Tokens = annotate("operator Foo() override { return Foo(); }", Style); + Tokens = annotate("operator Foo() override { return Foo(); }"); ASSERT_EQ(Tokens.size(), 13u) << Tokens; EXPECT_TOKEN(Tokens[0], tok::kw_operator, TT_FunctionDeclarationName); EXPECT_TOKEN(Tokens[2], tok::l_paren, TT_FunctionDeclarationLParen); EXPECT_TOKEN(Tokens[5], tok::l_brace, TT_FunctionLBrace); - Tokens = annotate("friend Bar::operator Foo();", Style); + Tokens = annotate("friend Bar::operator Foo();"); ASSERT_EQ(Tokens.size(), 9u) << Tokens; EXPECT_TOKEN(Tokens[3], tok::kw_operator, TT_FunctionDeclarationName); EXPECT_TOKEN(Tokens[5], tok::l_paren, TT_FunctionDeclarationLParen); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits