[clang] [Clang][OpenMP] Validate prefer_type fr()/attr() arguments in append_args clause (PR #212307)

via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 28 06:21:57 PDT 2026


https://github.com/ykhatav updated https://github.com/llvm/llvm-project/pull/212307

>From 109defc9c6eb461cc4534ac21978a65a425892cb Mon Sep 17 00:00:00 2001
From: "Khatavkar, Yashasvi" <yashasvi.khatavkar at intel.com>
Date: Mon, 27 Jul 2026 10:29:02 -0700
Subject: [PATCH 1/2] Validate prefer_type fr()/attr() arguments in append_args
 clause

---
 clang/lib/Parse/ParseOpenMP.cpp               |  5 +-
 clang/lib/Sema/SemaOpenMP.cpp                 | 90 +++++++++++--------
 ...riant_append_args_prefer_type_messages.cpp | 59 ++++++++++++
 3 files changed, 114 insertions(+), 40 deletions(-)
 create mode 100644 clang/test/OpenMP/declare_variant_append_args_prefer_type_messages.cpp

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index 045b704f5480c..aa3f48a96bee2 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -3752,8 +3752,9 @@ bool Parser::ParseOMPInteropInfo(OMPInteropInfo &InteropInfo,
   bool IsTargetSync = false;
 
   while (Tok.is(tok::identifier)) {
-    // Currently prefer_type is only allowed with 'init' and it must be first.
-    bool PreferTypeAllowed = Kind == OMPC_init && InteropInfo.Prefs.empty() &&
+    // prefer_type is allowed with 'init' and 'append_args' and must be first.
+    bool PreferTypeAllowed = (Kind == OMPC_init || Kind == OMPC_append_args) &&
+                             InteropInfo.Prefs.empty() &&
                              !IsTarget && !IsTargetSync;
     if (Tok.getIdentifierInfo()->isStr("target")) {
       // OpenMP 5.1 [2.15.1, interop Construct, Restrictions]
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 5b59eacb3eea8..b80354e394570 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -7842,6 +7842,49 @@ SemaOpenMP::checkOpenMPDeclareVariantFunction(SemaOpenMP::DeclGroupPtrTy DG,
   return std::make_pair(FD, cast<Expr>(DRE));
 }
 
+/// Check prefer_type fr()/attr() arguments in an OMPInteropInfo for validity.
+/// Returns true if all arguments are valid; emits a diagnostic and returns
+/// false on the first invalid argument.
+static bool checkPreferTypeArgs(SemaOpenMP &S, const OMPInteropInfo &Info) {
+  for (const OMPInteropPref &P : Info.Prefs) {
+    const Expr *E = P.Fr;
+    if (!E) {
+      assert(Info.HasPreferAttrs && "null Fr requires OMP 6.0 syntax");
+    } else if (!E->isValueDependent() && !E->isTypeDependent() &&
+               !E->isInstantiationDependent() &&
+               !E->containsUnexpandedParameterPack()) {
+      if (!E->isIntegerConstantExpr(S.getASTContext()) &&
+          !isa<StringLiteral>(E)) {
+        S.Diag(E->getExprLoc(), diag::err_omp_interop_prefer_type);
+        return false;
+      }
+    }
+    for (const Expr *A : P.Attrs) {
+      if (A->isValueDependent() || A->isTypeDependent() ||
+          A->isInstantiationDependent() ||
+          A->containsUnexpandedParameterPack())
+        continue;
+      const auto *SL = dyn_cast<StringLiteral>(A);
+      if (!SL) {
+        S.Diag(A->getExprLoc(), diag::err_omp_interop_attr_not_string);
+        return false;
+      }
+      if (!SL->getString().starts_with("ompx_")) {
+        S.Diag(A->getExprLoc(),
+               diag::err_omp_interop_attr_missing_ompx_prefix)
+            << SL->getString();
+        return false;
+      }
+      if (SL->getString().contains(',')) {
+        S.Diag(A->getExprLoc(), diag::err_omp_interop_attr_contains_comma)
+            << SL->getString();
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
 void SemaOpenMP::ActOnOpenMPDeclareVariantDirective(
     FunctionDecl *FD, Expr *VariantRef, OMPTraitInfo &TI,
     ArrayRef<Expr *> AdjustArgsNothing,
@@ -7921,6 +7964,13 @@ void SemaOpenMP::ActOnOpenMPDeclareVariantDirective(
     }
   }
 
+  // OpenMP 6.0 [16.1.3] Check prefer_type fr()/attr() arguments in
+  // append_args.
+  for (const OMPInteropInfo &Info : AppendArgs) {
+    if (!checkPreferTypeArgs(*this, Info))
+      return;
+  }
+
   auto *NewAttr = OMPDeclareVariantAttr::CreateImplicit(
       getASTContext(), VariantRef, &TI,
       const_cast<Expr **>(AdjustArgsNothing.data()), AdjustArgsNothing.size(),
@@ -19086,44 +19136,8 @@ OMPClause *SemaOpenMP::ActOnOpenMPInitClause(
   if (!isValidInteropVariable(SemaRef, InteropVar, VarLoc, OMPC_init))
     return nullptr;
 
-  // Check prefer_type values. fr() arguments are either string literals or
-  // constant integral expressions; null Fr is only valid in OMP 6.0.
-  // attr() arguments must be ext-string-literals with the 'ompx_' prefix
-  // (OpenMP 6.0 spec, section 16.1.3).
-  for (const OMPInteropPref &P : InteropInfo.Prefs) {
-    const Expr *E = P.Fr;
-    if (!E) {
-      assert(InteropInfo.HasPreferAttrs && "null Fr requires OMP 6.0 syntax");
-    } else if (!E->isValueDependent() && !E->isTypeDependent() &&
-               !E->isInstantiationDependent() &&
-               !E->containsUnexpandedParameterPack()) {
-      if (!E->isIntegerConstantExpr(getASTContext()) &&
-          !isa<StringLiteral>(E)) {
-        Diag(E->getExprLoc(), diag::err_omp_interop_prefer_type);
-        return nullptr;
-      }
-    }
-    for (const Expr *A : P.Attrs) {
-      if (A->isValueDependent() || A->isTypeDependent() ||
-          A->isInstantiationDependent() || A->containsUnexpandedParameterPack())
-        continue;
-      const auto *SL = dyn_cast<StringLiteral>(A);
-      if (!SL) {
-        Diag(A->getExprLoc(), diag::err_omp_interop_attr_not_string);
-        return nullptr;
-      }
-      if (!SL->getString().starts_with("ompx_")) {
-        Diag(A->getExprLoc(), diag::err_omp_interop_attr_missing_ompx_prefix)
-            << SL->getString();
-        return nullptr;
-      }
-      if (SL->getString().contains(',')) {
-        Diag(A->getExprLoc(), diag::err_omp_interop_attr_contains_comma)
-            << SL->getString();
-        return nullptr;
-      }
-    }
-  }
+  if (!checkPreferTypeArgs(*this, InteropInfo))
+    return nullptr;
 
   return OMPInitClause::Create(getASTContext(), InteropVar, InteropInfo,
                                StartLoc, LParenLoc, VarLoc, EndLoc);
diff --git a/clang/test/OpenMP/declare_variant_append_args_prefer_type_messages.cpp b/clang/test/OpenMP/declare_variant_append_args_prefer_type_messages.cpp
new file mode 100644
index 0000000000000..0fb9566faf3a3
--- /dev/null
+++ b/clang/test/OpenMP/declare_variant_append_args_prefer_type_messages.cpp
@@ -0,0 +1,59 @@
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=60 -std=c++11 -o - %s
+
+typedef void *omp_interop_t;
+
+void foo_v1(float *A, float *B, omp_interop_t IOp);
+
+// expected-error at +2 {{prefer_list item must be a string literal or constant integral expression}}
+#pragma omp declare variant(foo_v1) match(construct={dispatch}) \
+  append_args(interop(prefer_type({fr(1.0)}), target))
+void foo_fr_float(float *A, float *B) {}
+
+void bar_v1(float *A, omp_interop_t IOp);
+
+// expected-error at +2 {{attr() argument must be a string literal}}
+#pragma omp declare variant(bar_v1) match(construct={dispatch}) \
+  append_args(interop(prefer_type({attr(1)}), target))
+void bar_attr_int(float *A) {}
+
+void baz_v1(float *A, omp_interop_t IOp);
+
+// expected-error at +2 {{attr() argument 'cuda_prop' must start with the 'ompx_' prefix}}
+#pragma omp declare variant(baz_v1) match(construct={dispatch}) \
+  append_args(interop(prefer_type({attr("cuda_prop")}), target))
+void baz_attr_no_prefix(float *A) {}
+
+void qux_v1(float *A, omp_interop_t IOp);
+
+// expected-error at +2 {{attr() argument 'ompx_a,b' must not contain a comma}}
+#pragma omp declare variant(qux_v1) match(construct={dispatch}) \
+  append_args(interop(prefer_type({attr("ompx_a,b")}), target))
+void qux_attr_comma(float *A) {}
+
+// Valid cases -- no diagnostics expected.
+void valid_v1(float *A, omp_interop_t IOp);
+
+#pragma omp declare variant(valid_v1) match(construct={dispatch}) \
+  append_args(interop(prefer_type({fr("cuda")}), target))
+void valid_fr_string(float *A) {}
+
+void valid_v2(float *A, omp_interop_t IOp);
+
+#pragma omp declare variant(valid_v2) match(construct={dispatch}) \
+  append_args(interop(prefer_type({fr(1)}), target))
+void valid_fr_int(float *A) {}
+
+void valid_v3(float *A, omp_interop_t IOp);
+
+#pragma omp declare variant(valid_v3) match(construct={dispatch}) \
+  append_args(interop(prefer_type({attr("ompx_myattr")}), target))
+void valid_attr(float *A) {}
+
+// Template case: fr() argument becomes invalid at instantiation.
+template <typename T>
+void tmpl_v1(T *A, omp_interop_t IOp);
+
+// expected-error at +2 {{prefer_list item must be a string literal or constant integral expression}}
+#pragma omp declare variant(tmpl_v1<int>) match(construct={dispatch}) \
+  append_args(interop(prefer_type({fr(1.5)}), target))
+void tmpl_fr_invalid(int *A) {}

>From 38ba72302249d0de69f2f36f0554999a1ad6eecc Mon Sep 17 00:00:00 2001
From: "Khatavkar, Yashasvi" <yashasvi.khatavkar at intel.com>
Date: Mon, 27 Jul 2026 11:37:10 -0700
Subject: [PATCH 2/2] Fix formatting

---
 clang/lib/Parse/ParseOpenMP.cpp | 4 ++--
 clang/lib/Sema/SemaOpenMP.cpp   | 6 ++----
 2 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index aa3f48a96bee2..6d3668316590c 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -3754,8 +3754,8 @@ bool Parser::ParseOMPInteropInfo(OMPInteropInfo &InteropInfo,
   while (Tok.is(tok::identifier)) {
     // prefer_type is allowed with 'init' and 'append_args' and must be first.
     bool PreferTypeAllowed = (Kind == OMPC_init || Kind == OMPC_append_args) &&
-                             InteropInfo.Prefs.empty() &&
-                             !IsTarget && !IsTargetSync;
+                             InteropInfo.Prefs.empty() && !IsTarget &&
+                             !IsTargetSync;
     if (Tok.getIdentifierInfo()->isStr("target")) {
       // OpenMP 5.1 [2.15.1, interop Construct, Restrictions]
       // Each interop-type may be specified on an action-clause at most
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index b80354e394570..67e4cf0d0974f 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -7861,8 +7861,7 @@ static bool checkPreferTypeArgs(SemaOpenMP &S, const OMPInteropInfo &Info) {
     }
     for (const Expr *A : P.Attrs) {
       if (A->isValueDependent() || A->isTypeDependent() ||
-          A->isInstantiationDependent() ||
-          A->containsUnexpandedParameterPack())
+          A->isInstantiationDependent() || A->containsUnexpandedParameterPack())
         continue;
       const auto *SL = dyn_cast<StringLiteral>(A);
       if (!SL) {
@@ -7870,8 +7869,7 @@ static bool checkPreferTypeArgs(SemaOpenMP &S, const OMPInteropInfo &Info) {
         return false;
       }
       if (!SL->getString().starts_with("ompx_")) {
-        S.Diag(A->getExprLoc(),
-               diag::err_omp_interop_attr_missing_ompx_prefix)
+        S.Diag(A->getExprLoc(), diag::err_omp_interop_attr_missing_ompx_prefix)
             << SL->getString();
         return false;
       }



More information about the cfe-commits mailing list