Skip to content
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

[coroutine] Clang gives incorrect warnings #56768

Closed
debashish-ghosh opened this issue Jul 28, 2022 · 4 comments
Closed

[coroutine] Clang gives incorrect warnings #56768

debashish-ghosh opened this issue Jul 28, 2022 · 4 comments
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer coroutines C++20 coroutines

Comments

@debashish-ghosh
Copy link

debashish-ghosh commented Jul 28, 2022

In the following code snippet:

#include <coroutine>
struct coro {
  struct promise_type {
    coro get_return_object();
    constexpr auto initial_suspend() { return std::suspend_never{}; }
    constexpr auto final_suspend() noexcept { return std::suspend_always{}; }
    void unhandled_exception();
    void return_void();
  };
};
coro c(int i) {
  co_await (i = 0, std::suspend_always{});
}

clang gives a weird warning:

warning: multiple unsequenced modifications to 'i' [-Wunsequenced]
  co_await (i = 1, std::suspend_always());
              ^
1 warning generated.

Another example can be found here.

@EugeneZelenko EugeneZelenko added clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed new issue labels Jul 28, 2022
@ChuanqiXu9 ChuanqiXu9 added the coroutines C++20 coroutines label Jul 28, 2022
@ecatmur
Copy link

ecatmur commented Jul 28, 2022

It looks like this is caused by https://reviews.llvm.org/D115187 df2a4ea

the fix will be to skip CoroutineSuspendExpr in SemaChecking/CheckUnsequencedOperations, like we do in AnalyzeImplicitConversions.

@r-barnes
Copy link

r-barnes commented Dec 6, 2022

We observe what we think is the same issue when doing, eg,

co_await foo(attempts++)

@bcardosolopes
Copy link
Member

A patch is up in https://reviews.llvm.org/D142077

bcardosolopes added a commit that referenced this issue Feb 3, 2023
…enced

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.

#56768

Differential Revision: https://reviews.llvm.org/D142077
@bcardosolopes
Copy link
Member

CarlosAlbertoEnciso pushed a commit to SNSystems/llvm-debuginfo-analyzer that referenced this issue Feb 4, 2023
…enced

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.

llvm/llvm-project#56768

Differential Revision: https://reviews.llvm.org/D142077
skatrak pushed a commit to skatrak/llvm-project-rocm that referenced this issue Feb 10, 2023
…enced

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.

llvm/llvm-project#56768

Differential Revision: https://reviews.llvm.org/D142077
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer coroutines C++20 coroutines
Projects
Status: No status
Development

No branches or pull requests

6 participants