Skip to content

Conversation

@owenca
Copy link
Contributor

@owenca owenca commented Apr 9, 2025

Also keep the space between not and ::.

Based on the documentation, it can be argued that SpaceAfterLogicalNot doesn't cover the alternative operator not.

Closes #125465

Based on the documentation, it can be argued that SpaceAfterLogicalNot
doesn't cover the alternative operator `not`.

Closes llvm#125465
@llvmbot
Copy link
Member

llvmbot commented Apr 9, 2025

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Based on the documentation, it can be argued that SpaceAfterLogicalNot doesn't cover the alternative operator not.

Closes #125465


Full diff: https://github.com/llvm/llvm-project/pull/135035.diff

2 Files Affected:

  • (modified) clang/lib/Format/TokenAnnotator.cpp (+6-1)
  • (modified) clang/unittests/Format/FormatTest.cpp (+3-2)
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index bd54470dcba37..31f8a71256fe8 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -5472,7 +5472,12 @@ bool TokenAnnotator::spaceRequiredBefore(const AnnotatedLine &Line,
     // handled.
     if (Left.is(tok::amp) && Right.is(tok::r_square))
       return Style.SpacesInSquareBrackets;
-    return Style.SpaceAfterLogicalNot && Left.is(tok::exclaim);
+    if (Left.isNot(tok::exclaim))
+      return false;
+    if (Left.TokenText == "!")
+      return Style.SpaceAfterLogicalNot;
+    assert(Left.TokenText == "not");
+    return Right.isOneOf(tok::coloncolon, TT_UnaryOperator);
   }
 
   // If the next token is a binary operator or a selector name, we have
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 69c9ee1d1dcb2..f0e67c604cc4b 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -25393,8 +25393,10 @@ TEST_F(FormatTest, AlternativeOperators) {
   verifyFormat("%:define ABC abc"); // #define ABC abc
   verifyFormat("%:%:");             // ##
 
+  verifyFormat("return not ::f();");
+  verifyFormat("return not *foo;");
+
   verifyFormat("a = v(not;);\n"
-               "b = v(not+);\n"
                "c = v(not x);\n"
                "d = v(not 1);\n"
                "e = v(not 123.f);");
@@ -25402,7 +25404,6 @@ TEST_F(FormatTest, AlternativeOperators) {
   verifyNoChange("#define ASSEMBLER_INSTRUCTION_LIST(V)  \\\n"
                  "  V(and)                               \\\n"
                  "  V(not)                               \\\n"
-                 "  V(not!)                              \\\n"
                  "  V(other)",
                  getLLVMStyleWithColumns(40));
 }

@owenca owenca merged commit f344838 into llvm:main Apr 10, 2025
13 checks passed
@owenca owenca deleted the 125465 branch April 10, 2025 00:52
@owenca owenca added this to the Clang C++20 milestone Apr 10, 2025
@owenca
Copy link
Contributor Author

owenca commented Apr 10, 2025

/cherry-pick f344838

@llvmbot
Copy link
Member

llvmbot commented Apr 10, 2025

/cherry-pick f344838

Error: Command failed due to missing milestone.

@owenca
Copy link
Contributor Author

owenca commented Apr 10, 2025

/cherry-pick f344838

@llvmbot
Copy link
Member

llvmbot commented Apr 10, 2025

/pull-request #135118

@llvmbot llvmbot moved this from Needs Triage to Done in LLVM Release Status Apr 10, 2025
AllinLeeYL pushed a commit to AllinLeeYL/llvm-project that referenced this pull request Apr 10, 2025
…m#135035)

Also keep the space between `not` and `::`.

Based on the
[documentation](https://releases.llvm.org/20.1.0/tools/clang/docs/ClangFormatStyleOptions.html#spaceafterlogicalnot),
it can be argued that SpaceAfterLogicalNot doesn't cover the alternative
operator `not`.

Closes llvm#125465
swift-ci pushed a commit to swiftlang/llvm-project that referenced this pull request Apr 11, 2025
…m#135035)

Also keep the space between `not` and `::`.

Based on the
[documentation](https://releases.llvm.org/20.1.0/tools/clang/docs/ClangFormatStyleOptions.html#spaceafterlogicalnot),
it can be argued that SpaceAfterLogicalNot doesn't cover the alternative
operator `not`.

Closes llvm#125465

(cherry picked from commit f344838)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

[clang-format] Lack of space after 'not' operator since clang-format-19

4 participants