Unverified Commit 94e0fd09 authored by Corentin Jabot's avatar Corentin Jabot Committed by GitHub
Browse files

[Clang] No longer advertise support for coroutines on x86 windows. (#193456)

There are a large number of long standing issues with coroutines on
x86_32 windows as discussed here

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

- #59382
- #58556
- #58543
- #56989
- #193161
- #136481

As such this patches
  - No longer defines `__cpp_impl_coroutine` on that platform
  - Warn when using coroutines on that platform

The reason to not plainly error is that it would break too much valid
valid code, ie there are people who probably use coroutines sucessfully.

And we also want to keep testing on that platform, up until the point we
were to pull support completely.

Hopefully this is all temporary and someone will feel compelled to
improve the situation enough that we can unceremoniously revert this
patch.
parent 31ea083c
Loading
Loading
Loading
Loading
+4 −1
Original line number Diff line number Diff line
@@ -86,7 +86,10 @@ Clang Frontend Potentially Breaking Changes
  libraries will need to be recompiled, or used with
  (`--no-offload-new-driver`). This option will be removed in the next release.


- Clang no longer defines the ``__cpp_impl_coroutine`` feature test macro under the 32-bit x86 Microsoft ABI,
  as support for coroutines on this target is incomplete.
  When using coroutines on this target a warning is emmitted to indicate the lack of full support.
  That warning can be disabled with ``-Wno-coroutines-unsupported-target``. (see #GH59382)

Clang Python Bindings Potentially Breaking Changes
--------------------------------------------------
+3 −0
Original line number Diff line number Diff line
@@ -12878,6 +12878,9 @@ def note_redefinition_include_same_file : Note<
}
let CategoryName = "Coroutines Issue" in {
def warn_coroutines_x86_windows : Warning<
  "coroutines are not currently supported on the 32-bit x86 Microsoft ABI">,
  InGroup<DiagGroup<"coroutines-unsupported-target">>;
def err_return_in_coroutine : Error<
  "return statement not allowed in coroutine; did you mean 'co_return'?">;
def note_declared_coroutine_here : Note<
+9 −3
Original line number Diff line number Diff line
@@ -621,7 +621,8 @@ static void InitializeStandardPredefinedMacros(const TargetInfo &TI,
/// Initialize the predefined C++ language feature test macros defined in
/// ISO/IEC JTC1/SC22/WG21 (C++) SD-6: "SG10 Feature Test Recommendations".
static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
                                                 MacroBuilder &Builder) {
                                                 MacroBuilder &Builder,
                                                 const TargetInfo &TI) {
  // C++98 features.
  if (LangOpts.RTTI)
    Builder.defineMacro("__cpp_rtti", "199711L");
@@ -714,7 +715,12 @@ static void InitializeCPlusPlusFeatureTestMacros(const LangOptions &LangOpts,
    Builder.defineMacro("__cpp_consteval", "202211L");
    Builder.defineMacro("__cpp_constexpr_dynamic_alloc", "201907L");
    Builder.defineMacro("__cpp_constinit", "201907L");

    // Support for coroutines on 32-bit x86 Microsoft platforms is
    // incomplete, do not advertise it.
    if (!(TI.getCXXABI().isMicrosoft() && TI.getTriple().isX86_32()))
      Builder.defineMacro("__cpp_impl_coroutine", "201902L");

    Builder.defineMacro("__cpp_designated_initializers", "201707L");
    Builder.defineMacro("__cpp_impl_three_way_comparison", "201907L");
    // Intentionally to set __cpp_modules to 1.
@@ -980,7 +986,7 @@ static void InitializePredefinedMacros(const TargetInfo &TI,
                      Twine(TI.useSignedCharForObjCBool() ? "0" : "1"));

  if (LangOpts.CPlusPlus)
    InitializeCPlusPlusFeatureTestMacros(LangOpts, Builder);
    InitializeCPlusPlusFeatureTestMacros(LangOpts, Builder, TI);

  // darwin_constant_cfstrings controls this. This is also dependent
  // on other things like the runtime I believe.  This is set even for C code.
+8 −0
Original line number Diff line number Diff line
@@ -21,6 +21,7 @@
#include "clang/AST/IgnoreExpr.h"
#include "clang/AST/StmtCXX.h"
#include "clang/Basic/Builtins.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Lex/Preprocessor.h"
#include "clang/Sema/EnterExpressionEvaluationContext.h"
#include "clang/Sema/Initialization.h"
@@ -693,6 +694,13 @@ bool Sema::ActOnCoroutineBodyStart(Scope *SC, SourceLocation KWLoc,

  if (!checkCoroutineContext(*this, KWLoc, Keyword))
    return false;

  // Support for coroutines is not stable on 32 bits windows
  // Warn about it.
  if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
      Context.getTargetInfo().getTriple().isX86_32())
    Diag(KWLoc, diag::warn_coroutines_x86_windows);

  auto *ScopeInfo = getCurFunction();
  assert(ScopeInfo->CoroutinePromise);

+1 −1
Original line number Diff line number Diff line
// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -std=c++20 -verify %s
// RUN: %clang_analyze_cc1 -analyzer-checker=webkit.UncountedLambdaCapturesChecker -std=c++20 -verify %s -Wno-coroutines-unsupported-target
// expected-no-diagnostics

template<typename Arg>
Loading