MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: krasimir, rianquinn, JakeMerdichAMD, 
mitchell-stellar.
MyDeveloperDay added projects: clang, clang-format.

https://bugs.llvm.org/show_bug.cgi?id=45198

The following:

  template<
      typename T,
      enable_if_t<is_move_assignable<T>::value> = true,
      enable_if_t<is_move_constructible<T>::value> = true>
  constexpr void
  swap(T &lhs, T &rhs) noexcept(
      is_nothrow_move_constructible<T>::value && 
is_nothrow_move_assignable<T>::value)

Results in this:

  template<
      typename T,
      enable_if_t<is_move_assignable<T>::value> = true,
      enable_if_t<is_move_constructible<T>::value> = true>
  constexpr void
  swap(T &lhs, T &rhs) noexcept(
      is_nothrow_move_constructible<T>::value 
&&is_nothrow_move_assignable<T>::value)

This is because the `&&` in `is_nothrow_move_constructible<T>::value 
&&is_nothrow_move_assignable<T>::value` gets incorrectly determined to be a 
TT_PointerOrReference

This revision attempts to detect determine a cases where this cannot be true 
especially in a `noexcept` context where the result is expected to be boolean


https://reviews.llvm.org/D80041

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8005,6 +8005,8 @@
   verifyFormat("@[ [NSArray class] ];");
   verifyFormat("@[ [foo enum] ];");
 
+  verifyFormat("template <typename T> [[nodiscard]] int a() { return 1; }");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
@@ -8091,6 +8093,37 @@
   verifyFormat("#define A(a, b) (a && b)");
 }
 
+TEST_F(FormatTest, BinaryOperationsInANoExceptContext) {
+  verifyFormat("int f(int &&a) {}");
+  verifyFormat("template <typename T> void swap() noexcept(Bar<T> && Foo<T>);");
+  verifyFormat(
+      "template <typename T> void swap() noexcept(Bar<T>::value && Foo<T>);");
+  verifyFormat("template <typename T> void swap() noexcept(Bar<T>::value && "
+               "Foo<T>::value);");
+  verifyFormat("template <typename T> void foo(Bar<T>::value &&b);");
+  verifyFormat("template <typename T> void swap() noexcept(Bar<T>::value && "
+               "std::Foo<T>);");
+  verifyFormat(
+      "template <typename T> void foo(Bar<T>::value &&a, Foo<T> &&b);");
+  verifyFormat("template <typename T> void swap() noexcept(Bar<T> &&b);");
+  verifyFormat(
+      "template <typename T> void swap() noexcept(Bar<T> && std::Foo<T>);");
+  verifyFormat("template <typename T> void foo(Bar<T> &&a, Foo<T> &&b);");
+  verifyFormat("template <typename T> void swap() noexcept(Bar && b);");
+  verifyFormat(
+      "template <typename T> void swap() noexcept(Bar && std::Foo<T>);");
+  verifyFormat("template <typename T> void foo(Bar &&a, Foo<T> &&b);");
+  verifyFormat(
+      "template <typename T> void swap() noexcept(Bar<T>() && Foo<T>());");
+
+  verifyFormat("template <typename T> void swap() noexcept(a && b);");
+  verifyFormat("template <typename T> void swap() noexcept(/*A*/ a && b);");
+  verifyFormat("template <typename T> void swap() noexcept(a && b /*A*/);");
+  verifyFormat(
+      "template <typename T> void swap() noexcept(/*A*/ a && /*A*/ b);");
+  verifyFormat("template <typename T> void foo(Bar &&b);");
+}
+
 TEST_F(FormatTest, FormatsBinaryOperatorsPrecedingEquals) {
   verifyFormat("void f() {\n"
                "  x[aaaaaaaaa -\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1918,8 +1918,32 @@
         return TT_BinaryOperator;
     }
 
-    // It is very unlikely that we are going to find a pointer or reference type
-    // definition on the RHS of an assignment.
+    // Try to distinguish (A && B) from (A &&b), which is ambiguous without
+    // other semantic information
+    // However in a noexcept context where it is going to be a boolean
+    // operation noexcept(A && b) resulting in a binary operation
+    if (Tok.is(tok::ampamp) && PrevToken &&
+        PrevToken->isOneOf(tok::identifier, TT_TemplateCloser) && NextToken) {
+      const FormatToken *NextNextToken = NextToken->getNextNonComment();
+      const FormatToken *PrevPrevToken = PrevToken->getPreviousNonComment();
+      if (NextToken->is(tok::identifier)) {
+        if (NextNextToken && NextNextToken->isOneOf(tok::less, tok::coloncolon))
+          return TT_BinaryOperator;
+        if (PrevPrevToken) {
+          const FormatToken *PrevPrevPrevToken =
+              PrevPrevToken->getPreviousNonComment();
+          // We already know its `x y identifier && identifer z`
+          // just need to confirm x,y,z
+          if (PrevPrevPrevToken && PrevPrevPrevToken->is(tok::kw_noexcept) &&
+              PrevPrevToken->is(tok::l_paren) &&
+              NextNextToken->is(tok::r_paren))
+            return TT_BinaryOperator;
+        }
+      }
+    }
+
+    // It is very unlikely that we are going to find a pointer or reference
+    // type definition on the RHS of an assignment.
     if (IsExpression && !Contexts.back().CaretFound)
       return TT_BinaryOperator;
 
@@ -1969,12 +1993,13 @@
   bool AutoFound;
   const AdditionalKeywords &Keywords;
 
-  // Set of "<" tokens that do not open a template parameter list. If parseAngle
-  // determines that a specific token can't be a template opener, it will make
-  // same decision irrespective of the decisions for tokens leading up to it.
-  // Store this information to prevent this from causing exponential runtime.
+  // Set of "<" tokens that do not open a template parameter list. If
+  // parseAngle determines that a specific token can't be a template opener,
+  // it will make same decision irrespective of the decisions for tokens
+  // leading up to it. Store this information to prevent this from causing
+  // exponential runtime.
   llvm::SmallPtrSet<FormatToken *, 16> NonTemplateLess;
-};
+}; // namespace
 
 static const int PrecedenceUnaryOperator = prec::PointerToMember + 1;
 static const int PrecedenceArrowAndPeriod = prec::PointerToMember + 2;
@@ -1999,7 +2024,8 @@
     if (!Current || Precedence > PrecedenceArrowAndPeriod)
       return;
 
-    // Conditional expressions need to be parsed separately for proper nesting.
+    // Conditional expressions need to be parsed separately for proper
+    // nesting.
     if (Precedence == prec::Conditional) {
       parseConditionalExpr();
       return;
@@ -2042,8 +2068,8 @@
 
       // Consume scopes: (), [], <> and {}
       if (Current->opensScope()) {
-        // In fragment of a JavaScript template string can look like '}..${' and
-        // thus close a scope and open a new one at the same time.
+        // In fragment of a JavaScript template string can look like '}..${'
+        // and thus close a scope and open a new one at the same time.
         while (Current && (!Current->closesScope() || Current->opensScope())) {
           next();
           parse();
@@ -2178,7 +2204,7 @@
   FormatToken *Current;
 };
 
-} // end anonymous namespace
+} // namespace
 
 void TokenAnnotator::setCommentLineLevels(
     SmallVectorImpl<AnnotatedLine *> &Lines) {
@@ -2896,6 +2922,11 @@
     // No whitespace in x(/*foo=*/1), except for JavaScript.
     return Style.Language == FormatStyle::LK_JavaScript ||
            !Left.TokenText.endswith("=*/");
+
+  // Space between template and attribute.
+  // e.g. template <typename T> [[nodiscard]] ...
+  if (Left.is(TT_TemplateCloser) && Right.is(TT_AttributeSquare))
+    return true;
   if (Right.is(tok::l_paren)) {
     if ((Left.is(tok::r_paren) && Left.is(TT_AttributeParen)) ||
         (Left.is(tok::r_square) && Left.is(TT_AttributeSquare)))
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D80041: [clang-form... MyDeveloperDay via Phabricator via cfe-commits

Reply via email to