[clang] 0b8daee - [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

Bruno Cardoso Lopes via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 3 15:39:10 PST 2023


Author: Bruno Cardoso Lopes
Date: 2023-02-03T15:37:29-08:00
New Revision: 0b8daee028a87ab8a6f8fe54d2eb2d5b5c2babd4

URL: https://github.com/llvm/llvm-project/commit/0b8daee028a87ab8a6f8fe54d2eb2d5b5c2babd4
DIFF: https://github.com/llvm/llvm-project/commit/0b8daee028a87ab8a6f8fe54d2eb2d5b5c2babd4.diff

LOG: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

D115187 exposed CoroutineSuspendExpr's operand, which makes some nodes to show
up twice during the traversal, confusing the check for unsequenced operations.
Skip the operand since it's already handled as part of the common expression
and get rid of the misleading warnings.

https://github.com/llvm/llvm-project/issues/56768

Differential Revision: https://reviews.llvm.org/D142077

Added: 
    clang/test/SemaCXX/warn-unsequenced-coro.cpp

Modified: 
    clang/lib/Sema/SemaChecking.cpp
    clang/test/SemaCXX/Inputs/std-coroutine.h
    clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
    clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
    clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index efba0a8871c6c..d7830432099ad 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -15241,6 +15241,23 @@ class SequenceChecker : public ConstEvaluatedExprVisitor<SequenceChecker> {
     Base::VisitStmt(E);
   }
 
+  void VisitCoroutineSuspendExpr(const CoroutineSuspendExpr *CSE) {
+    for (auto *Sub : CSE->children()) {
+      const Expr *ChildExpr = dyn_cast_or_null<Expr>(Sub);
+      if (!ChildExpr)
+        continue;
+
+      if (ChildExpr == CSE->getOperand())
+        // Do not recurse over a CoroutineSuspendExpr's operand.
+        // The operand is also a subexpression of getCommonExpr(), and
+        // recursing into it directly could confuse object management
+        // for the sake of sequence tracking.
+        continue;
+
+      Visit(Sub);
+    }
+  }
+
   void VisitCastExpr(const CastExpr *E) {
     Object O = Object();
     if (E->getCastKind() == CK_LValueToRValue)

diff  --git a/clang/test/SemaCXX/Inputs/std-coroutine.h b/clang/test/SemaCXX/Inputs/std-coroutine.h
index 832ae7746f68b..0bc459d48ccc6 100644
--- a/clang/test/SemaCXX/Inputs/std-coroutine.h
+++ b/clang/test/SemaCXX/Inputs/std-coroutine.h
@@ -4,12 +4,23 @@
 
 namespace std {
 
+template<typename T> struct remove_reference       { typedef T type; };
+template<typename T> struct remove_reference<T &>  { typedef T type; };
+template<typename T> struct remove_reference<T &&> { typedef T type; };
+
+template<typename T>
+typename remove_reference<T>::type &&move(T &&t) noexcept;
+
+struct input_iterator_tag {};
+struct forward_iterator_tag : public input_iterator_tag {};
+
 template <class Ret, typename... T>
 struct coroutine_traits { using promise_type = typename Ret::promise_type; };
 
 template <class Promise = void>
 struct coroutine_handle {
   static coroutine_handle from_address(void *) noexcept;
+  static coroutine_handle from_promise(Promise &promise);
   constexpr void* address() const noexcept;
 };
 template <>

diff  --git a/clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp b/clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
index ae67f3aa819a8..97b4e687ed870 100644
--- a/clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
@@ -26,5 +26,5 @@ void test() {
   co_return; // expected-error {{mixed use of std and std::experimental namespaces for coroutine components}}
   // expected-warning at -1{{support for 'std::experimental::coroutine_traits' will be removed}}
   // expected-note at Inputs/std-coroutine-exp-namespace.h:8 {{'coroutine_traits' declared here}}
-  // expected-note at Inputs/std-coroutine.h:8 {{'coroutine_traits' declared here}}
+  // expected-note at Inputs/std-coroutine.h:18 {{'coroutine_traits' declared here}}
 }

diff  --git a/clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp b/clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
index a4f228a227838..7fec83fdd7c28 100644
--- a/clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
@@ -27,5 +27,5 @@ void test() {
   co_return; // expected-error {{mixed use of std and std::experimental namespaces for coroutine components}}
   // expected-warning at -1{{support for 'std::experimental::coroutine_traits' will be removed}}
   // expected-note at Inputs/std-coroutine-exp-namespace.h:8 {{'coroutine_traits' declared here}}
-  // expected-note at Inputs/std-coroutine.h:8 {{'coroutine_traits' declared here}}
+  // expected-note at Inputs/std-coroutine.h:18 {{'coroutine_traits' declared here}}
 }

diff  --git a/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp b/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
index 49e79b868b0f5..b09482d5b426b 100644
--- a/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
@@ -28,5 +28,5 @@ struct std::coroutine_traits<void> { using promise_type = promise_void; };
 void test() {
   co_return; // expected-error {{mixed use of std and std::experimental namespaces for coroutine components}}
   // expected-warning at -1{{support for 'std::experimental::coroutine_traits' will be removed}}
-  // expected-note at Inputs/std-coroutine.h:8 {{'coroutine_traits' declared here}}
+  // expected-note at Inputs/std-coroutine.h:18 {{'coroutine_traits' declared here}}
 }

diff  --git a/clang/test/SemaCXX/warn-unsequenced-coro.cpp b/clang/test/SemaCXX/warn-unsequenced-coro.cpp
new file mode 100644
index 0000000000000..56d2edcf30155
--- /dev/null
+++ b/clang/test/SemaCXX/warn-unsequenced-coro.cpp
@@ -0,0 +1,105 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fsyntax-only -verify -std=c++20 -I%S/Inputs -Wno-unused -Wno-uninitialized -Wunsequenced %s
+
+// expected-no-diagnostics
+
+#include "std-coroutine.h"
+
+typedef __PTRDIFF_TYPE__ ptr
diff _t;
+
+using namespace std;
+
+template<class T>
+struct Task {
+    struct promise_type {
+        Task<T> get_return_object() noexcept;
+        suspend_always initial_suspend() noexcept;
+        suspend_always final_suspend() noexcept;
+        void return_value(T);
+        void unhandled_exception();
+        auto yield_value(Task<T>) noexcept { return final_suspend(); }
+    };
+    bool await_ready() noexcept { return false; }
+    void await_suspend(coroutine_handle<>) noexcept {}
+    T await_resume();
+};
+
+template<>
+struct Task<void> {
+    struct promise_type {
+        Task<void> get_return_object() noexcept;
+        suspend_always initial_suspend() noexcept;
+        suspend_always final_suspend() noexcept;
+        void return_void() noexcept;
+        void unhandled_exception() noexcept;
+        auto yield_value(Task<void>) noexcept { return final_suspend(); }
+    };
+    bool await_ready() noexcept { return false; }
+    void await_suspend(coroutine_handle<>) noexcept {}
+    void await_resume() noexcept {}
+};
+
+template <typename T>
+class generator
+{
+  struct Promise
+  {
+    auto get_return_object() { return generator{*this}; }
+    auto initial_suspend() { return suspend_never{}; }
+    auto final_suspend() noexcept { return suspend_always{}; }
+    void unhandled_exception() {}
+    void return_void() {}
+
+    auto yield_value(T value)
+    {
+      value_ = std::move(value);
+      return suspend_always{};
+    }
+
+    T value_;
+  };
+
+  using Handle = coroutine_handle<Promise>;
+
+  struct sentinel{};
+  struct iterator
+  {
+    using iterator_category = input_iterator_tag;
+    using value_type = T;
+    using 
diff erence_type = ptr
diff _t;
+    using reference = T &;
+    using const_reference = const T &;
+    using pointer = T *;
+
+    iterator &operator++()
+    {
+      h_.resume();
+      return *this;
+    }
+    const_reference &operator*() const { return h_.promise().value_; }
+    bool operator!=(sentinel) { return !h_.done(); }
+
+    Handle h_;
+  };
+
+  explicit generator(Promise &p) : h_(Handle::from_promise(p)) {}
+  Handle h_;
+public:
+  using promise_type = Promise;
+  auto begin() { return iterator{h_}; }
+  auto end() { return sentinel{}; }
+};
+
+Task<void> c(int i) {
+  co_await (i = 0, std::suspend_always{});
+}
+
+generator<int> range(int start, int end)
+{
+  while (start < end)
+    co_yield start++;
+}
+
+Task<int> go(int const& val);
+Task<int> go1(int x) {
+  co_return co_await go(++x);
+}
\ No newline at end of file


        


More information about the cfe-commits mailing list