[clang] [clang-format] Insert a space between a keyword and a literal (PR #93632)

Owen Pan via cfe-commits cfe-commits at lists.llvm.org
Wed May 29 22:36:55 PDT 2024


https://github.com/owenca updated https://github.com/llvm/llvm-project/pull/93632

>From be3cb7662b789c7ffa61aab55c9390543cbab2ca Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Tue, 28 May 2024 19:36:36 -0700
Subject: [PATCH 1/2] [clang-format] Insert a space between a keyword and a
 literal

Fixes #93603.
---
 clang/lib/Format/FormatToken.h           | 23 +++++++++++++++++++++++
 clang/lib/Format/TokenAnnotator.cpp      |  8 ++++++--
 clang/lib/Format/UnwrappedLineParser.cpp |  9 +--------
 clang/unittests/Format/FormatTest.cpp    |  6 ++++++
 4 files changed, 36 insertions(+), 10 deletions(-)

diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index 8792f4c750748..f663a80cd3917 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -727,6 +727,29 @@ struct FormatToken {
     return isOneOf(tok::star, tok::amp, tok::ampamp);
   }
 
+  bool isAltOperator() const {
+    assert(!TokenText.empty());
+    if (!isalpha(TokenText[0]))
+      return false;
+
+    switch (Tok.getKind()) {
+    case tok::ampamp:
+    case tok::ampequal:
+    case tok::amp:
+    case tok::pipe:
+    case tok::tilde:
+    case tok::exclaim:
+    case tok::exclaimequal:
+    case tok::pipepipe:
+    case tok::pipeequal:
+    case tok::caret:
+    case tok::caretequal:
+      return true;
+    default:
+      return false;
+    }
+  }
+
   bool isUnaryOperator() const {
     switch (Tok.getKind()) {
     case tok::plus:
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 7c4c76a91f2c5..58a82e6daca73 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4846,8 +4846,12 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
         Right.is(TT_TemplateOpener)) {
       return true;
     }
-    if (Left.Tok.getIdentifierInfo() && Right.is(tok::numeric_constant))
-      return Right.TokenText[0] != '.';
+    if (Right.Tok.isLiteral()) {
+      if (Left.is(tok::identifier))
+        return Right.TokenText[0] != '.';
+      if (Left.isAltOperator())
+        return true;
+    }
   } else if (Style.isProto()) {
     if (Right.is(tok::period) &&
         Left.isOneOf(Keywords.kw_optional, Keywords.kw_required,
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index b6f7567adc140..b2f8108edeaec 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1410,13 +1410,6 @@ void UnwrappedLineParser::readTokenWithJavaScriptASI() {
   }
 }
 
-static bool isAltOperator(const FormatToken &Tok) {
-  return isalpha(Tok.TokenText[0]) &&
-         Tok.isOneOf(tok::ampamp, tok::ampequal, tok::amp, tok::pipe,
-                     tok::tilde, tok::exclaim, tok::exclaimequal, tok::pipepipe,
-                     tok::pipeequal, tok::caret, tok::caretequal);
-}
-
 void UnwrappedLineParser::parseStructuralElement(
     const FormatToken *OpeningBrace, IfStmtKind *IfKind,
     FormatToken **IfLeftBrace, bool *HasDoWhile, bool *HasLabel) {
@@ -1699,7 +1692,7 @@ void UnwrappedLineParser::parseStructuralElement(
   for (const bool InRequiresExpression =
            OpeningBrace && OpeningBrace->is(TT_RequiresExpressionLBrace);
        !eof();) {
-    if (IsCpp && isAltOperator(*FormatTok)) {
+    if (IsCpp && FormatTok->isAltOperator()) {
       if (auto *Next = Tokens->peekNextToken(/*SkipComment=*/true);
           Next && Next->isBinaryOperator()) {
         FormatTok->Tok.setKind(tok::identifier);
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 76eb2b6cd994c..59f1ff6a4b296 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -27452,6 +27452,12 @@ TEST_F(FormatTest, AlignUTFCommentsAndStringLiterals) {
                Style);
 }
 
+TEST_F(FormatTest, SpaceBetweenKeywordAndLiteral) {
+  verifyFormat("return .5;");
+  verifyFormat("return not '5';");
+  verifyFormat("return sizeof \"5\";");
+}
+
 } // namespace
 } // namespace test
 } // namespace format

>From 07276bc90bbd0f6c04e5a374e53cd1309ce9c5f0 Mon Sep 17 00:00:00 2001
From: Owen Pan <owenpiano at gmail.com>
Date: Wed, 29 May 2024 22:33:00 -0700
Subject: [PATCH 2/2] Improve code in TokenAnnotator.cpp and give
 `isAltOperator` a better name.

---
 clang/lib/Format/FormatToken.h           |  2 +-
 clang/lib/Format/TokenAnnotator.cpp      | 12 ++++++------
 clang/lib/Format/UnwrappedLineParser.cpp |  2 +-
 3 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Format/FormatToken.h b/clang/lib/Format/FormatToken.h
index f663a80cd3917..e4a4f27e502b1 100644
--- a/clang/lib/Format/FormatToken.h
+++ b/clang/lib/Format/FormatToken.h
@@ -727,7 +727,7 @@ struct FormatToken {
     return isOneOf(tok::star, tok::amp, tok::ampamp);
   }
 
-  bool isAltOperator() const {
+  bool isCppAlternativeOperatorKeyword() const {
     assert(!TokenText.empty());
     if (!isalpha(TokenText[0]))
       return false;
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 58a82e6daca73..26c0aa36bdcb6 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -4846,12 +4846,12 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
         Right.is(TT_TemplateOpener)) {
       return true;
     }
-    if (Right.Tok.isLiteral()) {
-      if (Left.is(tok::identifier))
-        return Right.TokenText[0] != '.';
-      if (Left.isAltOperator())
-        return true;
-    }
+    // C++ Core Guidelines suppression tag, e.g. `[[suppress(type.5)]]`.
+    if (Left.is(tok::identifier) && Right.is(tok::numeric_constant))
+      return Right.TokenText[0] != '.';
+    // `Left` is a keyword (including C++ alternative operator) or identifier.
+    if (Left.Tok.getIdentifierInfo() && Right.Tok.isLiteral())
+      return true;
   } else if (Style.isProto()) {
     if (Right.is(tok::period) &&
         Left.isOneOf(Keywords.kw_optional, Keywords.kw_required,
diff --git a/clang/lib/Format/UnwrappedLineParser.cpp b/clang/lib/Format/UnwrappedLineParser.cpp
index b2f8108edeaec..9b866ba0938e1 100644
--- a/clang/lib/Format/UnwrappedLineParser.cpp
+++ b/clang/lib/Format/UnwrappedLineParser.cpp
@@ -1692,7 +1692,7 @@ void UnwrappedLineParser::parseStructuralElement(
   for (const bool InRequiresExpression =
            OpeningBrace && OpeningBrace->is(TT_RequiresExpressionLBrace);
        !eof();) {
-    if (IsCpp && FormatTok->isAltOperator()) {
+    if (IsCpp && FormatTok->isCppAlternativeOperatorKeyword()) {
       if (auto *Next = Tokens->peekNextToken(/*SkipComment=*/true);
           Next && Next->isBinaryOperator()) {
         FormatTok->Tok.setKind(tok::identifier);



More information about the cfe-commits mailing list