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

[OpenMP] Move unsupported structured bindings diagnostic #80216

Merged
merged 1 commit into from
Feb 1, 2024

Conversation

mikerice1969
Copy link
Contributor

Move the diagnostic so it fires only when doing an OpenMP capture, not for non-OpenMP captures. This allows non-OpenMP code to work when using OpenMP elsewhere, such as the code reported in
#66999.

Move the diagnostic so it fires only when doing an OpenMP capture, not
for non-OpenMP captures. This allows non-OpenMP code to work when using
OpenMP elsewhere, such as the code reported in
llvm#66999.
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 31, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 31, 2024

@llvm/pr-subscribers-clang

Author: Mike Rice (mikerice1969)

Changes

Move the diagnostic so it fires only when doing an OpenMP capture, not for non-OpenMP captures. This allows non-OpenMP code to work when using OpenMP elsewhere, such as the code reported in
#66999.


Full diff: https://github.com/llvm/llvm-project/pull/80216.diff

2 Files Affected:

  • (modified) clang/lib/Sema/SemaExpr.cpp (+8-10)
  • (modified) clang/test/SemaCXX/decomposition-openmp.cpp (+24-5)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index abe300ecc5431..d15278bce5a6b 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -19528,16 +19528,6 @@ static bool captureInLambda(LambdaScopeInfo *LSI, ValueDecl *Var,
     ByRef = (LSI->ImpCaptureStyle == LambdaScopeInfo::ImpCap_LambdaByref);
   }
 
-  BindingDecl *BD = dyn_cast<BindingDecl>(Var);
-  // FIXME: We should support capturing structured bindings in OpenMP.
-  if (!Invalid && BD && S.LangOpts.OpenMP) {
-    if (BuildAndDiagnose) {
-      S.Diag(Loc, diag::err_capture_binding_openmp) << Var;
-      S.Diag(Var->getLocation(), diag::note_entity_declared_at) << Var;
-    }
-    Invalid = true;
-  }
-
   if (BuildAndDiagnose && S.Context.getTargetInfo().getTriple().isWasm() &&
       CaptureType.getNonReferenceType().isWebAssemblyReferenceType()) {
     S.Diag(Loc, diag::err_wasm_ca_reference) << 0;
@@ -19879,6 +19869,14 @@ bool Sema::tryCaptureVariable(
         // just break here. Similarly, global variables that are captured in a
         // target region should not be captured outside the scope of the region.
         if (RSI->CapRegionKind == CR_OpenMP) {
+          // FIXME: We should support capturing structured bindings in OpenMP.
+          if (isa<BindingDecl>(Var)) {
+            if (BuildAndDiagnose) {
+              Diag(ExprLoc, diag::err_capture_binding_openmp) << Var;
+              Diag(Var->getLocation(), diag::note_entity_declared_at) << Var;
+            }
+            return true;
+          }
           OpenMPClauseKind IsOpenMPPrivateDecl = isOpenMPPrivateDecl(
               Var, RSI->OpenMPLevel, RSI->OpenMPCaptureLevel);
           // If the variable is private (i.e. not captured) and has variably
diff --git a/clang/test/SemaCXX/decomposition-openmp.cpp b/clang/test/SemaCXX/decomposition-openmp.cpp
index 28afc39800399..2185f3db83d4e 100644
--- a/clang/test/SemaCXX/decomposition-openmp.cpp
+++ b/clang/test/SemaCXX/decomposition-openmp.cpp
@@ -1,13 +1,32 @@
-
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++20 -fopenmp %s
 
-// FIXME: OpenMP should support capturing structured bindings
+// Okay, not an OpenMP capture.
 auto f() {
   int i[2] = {};
-  auto [a, b] = i; // expected-note 2{{declared here}}
+  auto [a, b] = i;
   return [=, &a] {
-    // expected-error@-1 {{capturing a structured binding is not yet supported in OpenMP}}
     return a + b;
-    // expected-error@-1 {{capturing a structured binding is not yet supported in OpenMP}}
   };
 }
+
+// Okay, not an OpenMP capture.
+void foo(int);
+void g() {
+  #pragma omp parallel
+  {
+    int i[2] = {};
+    auto [a, b] = i;
+    auto L = [&] { foo(a+b); };
+  }
+}
+
+// FIXME: OpenMP should support capturing structured bindings
+void h() {
+  int i[2] = {};
+  auto [a, b] = i; // expected-note 2{{declared here}}
+  #pragma omp parallel
+  {
+    // expected-error@+1 2{{capturing a structured binding is not yet supported in OpenMP}}
+    foo(a + b);
+  }
+}

Copy link
Member

@alexey-bataev alexey-bataev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG

@mikerice1969 mikerice1969 merged commit de1ea78 into llvm:main Feb 1, 2024
6 checks passed
@mikerice1969 mikerice1969 deleted the move-struct-binding-msg branch February 1, 2024 18:07
carlosgalvezp pushed a commit to carlosgalvezp/llvm-project that referenced this pull request Feb 1, 2024
Move the diagnostic so it fires only when doing an OpenMP capture, not
for non-OpenMP captures. This allows non-OpenMP code to work when using
OpenMP elsewhere, such as the code reported in
llvm#66999.
agozillon pushed a commit to agozillon/llvm-project that referenced this pull request Feb 5, 2024
Move the diagnostic so it fires only when doing an OpenMP capture, not
for non-OpenMP captures. This allows non-OpenMP code to work when using
OpenMP elsewhere, such as the code reported in
llvm#66999.
@loriab loriab mentioned this pull request Jul 3, 2024
16 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants