Skip to content

link error when compiling coroutine code. #78290

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
hokein opened this issue Jan 16, 2024 · 10 comments · Fixed by #78589
Closed

link error when compiling coroutine code. #78290

hokein opened this issue Jan 16, 2024 · 10 comments · Fixed by #78589
Assignees
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" coroutines C++20 coroutines

Comments

@hokein
Copy link
Collaborator

hokein commented Jan 16, 2024

See https://godbolt.org/z/5nvhqhsz9 for a minimal testcase.

The following code is compiled with gcc, but clang emits a link error (in function Foo::CoroutineBody(): <source>:60: undefined reference to CoroutinePromise<void>::CoroutinePromise<Foo&>(Foo&)) when linking to a binary. We seem to miss a symbol.

#include <coroutine>

template <typename Final>
class Gen;

template <typename Final>
class CoroutinePromise final {
 public:
  template <typename... Args>
  explicit CoroutinePromise(Args&&... args) {}

  Gen<Final> get_return_object() { return Gen<Final>(my_handle()); }

  std::coroutine_handle<CoroutinePromise> my_handle() {
    return std::coroutine_handle<CoroutinePromise>::from_promise(*this);
  }
  void unhandled_exception() {}
  void return_void() {}

  template <typename U>
  auto await_transform(Gen<U> gen) {
    struct Awaitable {
      bool await_ready() { return false; }
      std::coroutine_handle<> await_suspend(
          const std::coroutine_handle<> handle_in) {
        return handle_in;
      }
      decltype(auto) await_resume() {}
    };
    return Awaitable();
  }

  std::suspend_always initial_suspend() { return {}; }
  std::suspend_always final_suspend() noexcept { return {}; }
};


template <typename Final>
class Gen final {
 public:
  using final_result_type = Final;
  using promise_type =CoroutinePromise<Final>;
  explicit Gen(const std::coroutine_handle<promise_type> handle)
      : handle_(handle) {}

  std::coroutine_handle<promise_type> handle_;  
};

class Foo  {
 public:
  Gen<void> TestBody2() {
    auto s =  [&]() -> Gen<void> { co_await this->CoroutineBody(); }();
    return s;
  }
 private:
  Gen<void> CoroutineBody();
};

Gen<void> Foo::CoroutineBody() {
  if constexpr (0) // remove it make clang compile.
    co_return;
}

int main() {
  return 0;
}
@hokein hokein added clang Clang issues not falling into any other category c++20 coroutines C++20 coroutines labels Jan 16, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 16, 2024

@llvm/issue-subscribers-c-20

Author: Haojian Wu (hokein)

See https://godbolt.org/z/rc37E78EM for a minimal testcase.

The following code is compiled with gcc, but clang emits a link error (in function Foo::CoroutineBody(): &lt;source&gt;:60: undefined reference to CoroutinePromise&lt;void&gt;::CoroutinePromise&lt;Foo&amp;&gt;(Foo&amp;)) when linking to a binary. We seem to miss a symbol.

#include &lt;coroutine&gt;

template &lt;typename Final&gt;
class Gen;

template &lt;typename Final&gt;
class CoroutinePromise final {
 public:
  template &lt;typename... Args&gt;
  explicit CoroutinePromise(Args&amp;&amp;... args) {}

  Gen&lt;Final&gt; get_return_object() { return Gen&lt;Final&gt;(my_handle()); }

  std::coroutine_handle&lt;CoroutinePromise&gt; my_handle() {
    return std::coroutine_handle&lt;CoroutinePromise&gt;::from_promise(*this);
  }
  void unhandled_exception() {}
  void return_void() {}

  template &lt;typename U&gt;
  auto await_transform(Gen&lt;U&gt; gen) {
    struct Awaitable {
      bool await_ready() { return false; }
      std::coroutine_handle&lt;&gt; await_suspend(
          const std::coroutine_handle&lt;&gt; handle_in) {
        return handle_in;
      }
      decltype(auto) await_resume() {}
    };
    return Awaitable();
  }

  std::suspend_always initial_suspend() { return {}; }
  std::suspend_always final_suspend() noexcept { return {}; }
};


template &lt;typename Final&gt;
class Gen final {
 public:
  using final_result_type = Final;
  using promise_type =CoroutinePromise&lt;Final&gt;;
  explicit Gen(const std::coroutine_handle&lt;promise_type&gt; handle)
      : handle_(handle) {}

  std::coroutine_handle&lt;promise_type&gt; handle_;  
};

class Foo  {
 public:
  Gen&lt;void&gt; TestBody2() {
    auto s = 
        [&amp;]() -&gt; Gen&lt;void&gt; { co_await this-&gt;CoroutineBody(); co_return; };
    return s();
  }
 private:
  Gen&lt;void&gt; CoroutineBody();
};

Gen&lt;void&gt; Foo ::CoroutineBody() {
  if constexpr (0) // remove it will make clang compile.
    co_await []() -&gt; Gen&lt;void&gt; { co_return; }();
}

int main() {
  return 0;
}

@llvmbot
Copy link
Member

llvmbot commented Jan 16, 2024

@llvm/issue-subscribers-coroutines

Author: Haojian Wu (hokein)

See https://godbolt.org/z/rc37E78EM for a minimal testcase.

The following code is compiled with gcc, but clang emits a link error (in function Foo::CoroutineBody(): &lt;source&gt;:60: undefined reference to CoroutinePromise&lt;void&gt;::CoroutinePromise&lt;Foo&amp;&gt;(Foo&amp;)) when linking to a binary. We seem to miss a symbol.

#include &lt;coroutine&gt;

template &lt;typename Final&gt;
class Gen;

template &lt;typename Final&gt;
class CoroutinePromise final {
 public:
  template &lt;typename... Args&gt;
  explicit CoroutinePromise(Args&amp;&amp;... args) {}

  Gen&lt;Final&gt; get_return_object() { return Gen&lt;Final&gt;(my_handle()); }

  std::coroutine_handle&lt;CoroutinePromise&gt; my_handle() {
    return std::coroutine_handle&lt;CoroutinePromise&gt;::from_promise(*this);
  }
  void unhandled_exception() {}
  void return_void() {}

  template &lt;typename U&gt;
  auto await_transform(Gen&lt;U&gt; gen) {
    struct Awaitable {
      bool await_ready() { return false; }
      std::coroutine_handle&lt;&gt; await_suspend(
          const std::coroutine_handle&lt;&gt; handle_in) {
        return handle_in;
      }
      decltype(auto) await_resume() {}
    };
    return Awaitable();
  }

  std::suspend_always initial_suspend() { return {}; }
  std::suspend_always final_suspend() noexcept { return {}; }
};


template &lt;typename Final&gt;
class Gen final {
 public:
  using final_result_type = Final;
  using promise_type =CoroutinePromise&lt;Final&gt;;
  explicit Gen(const std::coroutine_handle&lt;promise_type&gt; handle)
      : handle_(handle) {}

  std::coroutine_handle&lt;promise_type&gt; handle_;  
};

class Foo  {
 public:
  Gen&lt;void&gt; TestBody2() {
    auto s = 
        [&amp;]() -&gt; Gen&lt;void&gt; { co_await this-&gt;CoroutineBody(); co_return; };
    return s();
  }
 private:
  Gen&lt;void&gt; CoroutineBody();
};

Gen&lt;void&gt; Foo ::CoroutineBody() {
  if constexpr (0) // remove it will make clang compile.
    co_await []() -&gt; Gen&lt;void&gt; { co_return; }();
}

int main() {
  return 0;
}

@usx95
Copy link
Contributor

usx95 commented Jan 16, 2024

Interestingly, turning TestBody2 into a coroutine also makes it work.

    Gen<void> TestBody2() {
        auto s = [&]() -> Gen<void> { co_await this->CoroutineBody(); }();
        co_return co_await s;
    }

@hokein
Copy link
Collaborator Author

hokein commented Jan 17, 2024

Reduced a bit more:

#include <coroutine>

class Gen;

class Promise {
 public:
  explicit  Promise() {}
  template <typename Arg>
  explicit Promise(const Arg& args) {}


  Gen get_return_object();

  std::coroutine_handle<Promise> my_handle() {
    return std::coroutine_handle<Promise>::from_promise(*this);
  }

  void unhandled_exception() {}
  void return_void() {}

  std::suspend_always initial_suspend() { return {}; }
  std::suspend_always final_suspend() noexcept { return {}; }
};

class Gen final {
 public:
  using promise_type =Promise;
  explicit Gen(const std::coroutine_handle<promise_type> handle)
      : handle_(handle) {}

  std::coroutine_handle<promise_type> handle_;  
};

Gen Promise::get_return_object() { return Gen(my_handle()); }

class Foo  {
 private:
  Gen f();
};

Gen Foo::f() {
  if constexpr (0) // removing it makes clang compile.
    co_return;
}

int main() {
  return 0;
}

The template constructor of Promise is:

FunctionTemplateDecl 0x55cbcff5cbd0 <line:11:3, line:12:38> col:12 Promise
| |-TemplateTypeParmDecl 0x55cbcff5c940 <line:11:13, col:22> col:22 referenced typename depth 0 index 0 Arg
| |-CXXConstructorDecl 0x55cbcff5cb18 <line:12:3, col:38> col:12 Promise 'void (const Arg &)' implicit-inline
| | |-ParmVarDecl 0x55cbcff5ca28 <col:20, col:31> col:31 args 'const Arg &'
| | `-CompoundStmt 0x55cbcff5d6c8 <col:37, col:38>
| `-CXXConstructorDecl 0x55cbcff623e8 <col:3, col:38> col:12 referenced Promise 'void (const Foo &)' implicit_instantiation implicit-inline
|   |-TemplateArgument type 'Foo'
|   | `-RecordType 0x55cbcff60d90 'Foo'
|   |   `-CXXRecord 0x55cbcff60ce8 'Foo'
|   `-ParmVarDecl 0x55cbcff622d8 <col:20, col:31> col:31 args 'const Foo &'

We somehow miss the function body for the template specialization void (const Foo &)this causes the undefined symbol error during linking. (Removing the constexpr 0 branch will bring the function body back).

@usx95 usx95 self-assigned this Jan 17, 2024
@usx95
Copy link
Contributor

usx95 commented Jan 17, 2024

Reduced more: Without use of member coroutines
https://godbolt.org/z/hfEzKEhGd

#include <coroutine>

class Gen {
   public:
    class promise_type {
       public:
        template <typename... Args>
        explicit promise_type(Args&&... args) {}

        Gen get_return_object() { return {}; }

        void unhandled_exception() {}
        void return_void() {}
        std::suspend_always await_transform(Gen gen) { return {}; }

        std::suspend_always initial_suspend() { return {}; }
        std::suspend_always final_suspend() noexcept { return {}; }
    };
};

Gen CoroutineBody() {
    if constexpr (0)  // remove it make clang compile.
        co_await Gen{};
    co_await Gen{};
}

int main() { return 0; }

As noticed earlier, CXXConstructorDecl for promise_type is marked as referenced instead of used and does not have the body attached.

Adding a co_await before if constexpr also resolves the issue. It breaks if there is no coroutine statement before if constexpr. https://godbolt.org/z/Mjr7Mn6G9

Gen CoroutineBody() {
    co_await Gen{};
    if constexpr (0) {  // remove it make clang compile.
        co_await Gen{};
    }
}

@usx95
Copy link
Contributor

usx95 commented Jan 17, 2024

This happens because the promise object is built when we see the first coroutine statement which is present in ExpressionEvaluationContext::DiscardedStatement context due to if constexpr (0). This makes clang believe that the promise constructor is only odr-used and not really used.

I think the solution can be the coroutine body should be built independently in a separate context irrespective of the context of the first coroutine statement.

@ChuanqiXu9
Copy link
Member

I think the solution can be the coroutine body should be built independently in a separate context irrespective of the context of the first coroutine statement.

I didn't understand this fully. What is the meaning of the context here?

@usx95
Copy link
Contributor

usx95 commented Jan 18, 2024

I am referring to the expression evaluation context. This is set to DiscardStatement here. The promise object built in ActOnCoroutineBodyStart (source) is under the same context.
This sets the OdrUseContext for the promise object constructor to be OdrUseContext::FormallyOdrUsed instead of correct OdrUseContext::Used
(used here)

@ChuanqiXu9
Copy link
Member

I am referring to the expression evaluation context. This is set to DiscardStatement here. The promise object built in ActOnCoroutineBodyStart (source) is under the same context. This sets the OdrUseContext for the promise object constructor to be OdrUseContext::FormallyOdrUsed instead of correct OdrUseContext::Used (used here)

Thanks. Got it. The proposed solution sounds proper.

@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed clang Clang issues not falling into any other category labels Jan 18, 2024
@llvmbot
Copy link
Member

llvmbot commented Jan 18, 2024

@llvm/issue-subscribers-clang-frontend

Author: Haojian Wu (hokein)

See https://godbolt.org/z/5nvhqhsz9 for a minimal testcase.

The following code is compiled with gcc, but clang emits a link error (in function Foo::CoroutineBody(): &lt;source&gt;:60: undefined reference to CoroutinePromise&lt;void&gt;::CoroutinePromise&lt;Foo&amp;&gt;(Foo&amp;)) when linking to a binary. We seem to miss a symbol.

#include &lt;coroutine&gt;

template &lt;typename Final&gt;
class Gen;

template &lt;typename Final&gt;
class CoroutinePromise final {
 public:
  template &lt;typename... Args&gt;
  explicit CoroutinePromise(Args&amp;&amp;... args) {}

  Gen&lt;Final&gt; get_return_object() { return Gen&lt;Final&gt;(my_handle()); }

  std::coroutine_handle&lt;CoroutinePromise&gt; my_handle() {
    return std::coroutine_handle&lt;CoroutinePromise&gt;::from_promise(*this);
  }
  void unhandled_exception() {}
  void return_void() {}

  template &lt;typename U&gt;
  auto await_transform(Gen&lt;U&gt; gen) {
    struct Awaitable {
      bool await_ready() { return false; }
      std::coroutine_handle&lt;&gt; await_suspend(
          const std::coroutine_handle&lt;&gt; handle_in) {
        return handle_in;
      }
      decltype(auto) await_resume() {}
    };
    return Awaitable();
  }

  std::suspend_always initial_suspend() { return {}; }
  std::suspend_always final_suspend() noexcept { return {}; }
};


template &lt;typename Final&gt;
class Gen final {
 public:
  using final_result_type = Final;
  using promise_type =CoroutinePromise&lt;Final&gt;;
  explicit Gen(const std::coroutine_handle&lt;promise_type&gt; handle)
      : handle_(handle) {}

  std::coroutine_handle&lt;promise_type&gt; handle_;  
};

class Foo  {
 public:
  Gen&lt;void&gt; TestBody2() {
    auto s =  [&amp;]() -&gt; Gen&lt;void&gt; { co_await this-&gt;CoroutineBody(); }();
    return s;
  }
 private:
  Gen&lt;void&gt; CoroutineBody();
};

Gen&lt;void&gt; Foo::CoroutineBody() {
  if constexpr (0) // remove it make clang compile.
    co_return;
}

int main() {
  return 0;
}

@usx95 usx95 moved this to In Progress in C++ 20 in Clang Jan 18, 2024
usx95 added a commit that referenced this issue Jan 19, 2024
Fixes: #78290

See the bug for more context.
```cpp
Gen ACoroutine() {
  if constexpr (0) // remove it make clang compile.
    co_return;
  co_await Gen{};
}
```
We miss symbol of ctor of promise_type if the first coroutine statement
happens to be inside the disabled branch of `if constexpr`.

This happens because the promise object is built when we see the first
coroutine statement which is present in
`ExpressionEvaluationContext::DiscardedStatement` context due to `if
constexpr (0)`. This makes clang believe that the promise constructor is
only odr-used and not really "used".

The expr evaluation context for the coroutine body should not be related
to the context in which the first coroutine statement appears. We
override the context to `PotentiallyEvaluated`.

---------

Co-authored-by: cor3ntin <[email protected]>
@github-project-automation github-project-automation bot moved this from In Progress to Done in C++ 20 in Clang Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++20 clang:frontend Language frontend issues, e.g. anything involving "Sema" coroutines C++20 coroutines
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

5 participants