[llvm-branch-commits] [clang] [Clang][OpenMP] Add parsing/sema for dims modifier in num_threads (PR #208353)

[Kevin Sala Penades via llvm-branch-commits]

================
@@ -17341,43 +17341,62 @@ static std::string getListOfPossibleValues(OpenMPClauseKind K, unsigned First,
 }
 
 OMPClause *SemaOpenMP::ActOnOpenMPNumThreadsClause(
-    OpenMPNumThreadsClauseModifier Modifier, Expr *NumThreads,
-    SourceLocation StartLoc, SourceLocation LParenLoc,
-    SourceLocation ModifierLoc, SourceLocation EndLoc) {
-  assert((ModifierLoc.isInvalid() || getLangOpts().OpenMP >= 60) &&
-         "Unexpected num_threads modifier in OpenMP < 60.");
-
-  if (ModifierLoc.isValid() && Modifier == OMPC_NUMTHREADS_unknown) {
-    std::string Values = getListOfPossibleValues(OMPC_num_threads, /*First=*/0,
-                                                 OMPC_NUMTHREADS_unknown);
-    Diag(ModifierLoc, diag::err_omp_unexpected_clause_value)
-        << Values << getOpenMPClauseNameForDiag(OMPC_num_threads);
-    return nullptr;
+    ArrayRef<Expr *> VarList,
+    OpenMPNumThreadsClauseModifier PrescriptivenessModifier,
+    SourceLocation PrescriptivenessModifierLoc,
+    OpenMPNumThreadsClauseModifier DimsModifier, Expr *DimsModifierExpr,
+    SourceLocation DimsModifierLoc, SourceLocation StartLoc,
+    SourceLocation LParenLoc, SourceLocation EndLoc) {
+  SmallVector<Expr *, 3> Vars(VarList.begin(), VarList.end());
+  for (Expr *&ValExpr : Vars) {
+    // OpenMP [2.5, Restrictions]
+    //  The num_threads expression must evaluate to a positive integer value.
+    if (!isNonNegativeIntegerValue(ValExpr, SemaRef, OMPC_num_threads,
+                                   /*StrictlyPositive=*/true))
+      return nullptr;
   }
----------------
kevinsala wrote:

Unknown identifiers in the modifiers are already handled by the parsing side. We only store modifiers and their locations if they are recognized. Unknown identifiers will fail when being parsed as part of the varlist in `ParseOpenMPVarList`.

In the sema side, I can add the asserts below. If these conditions are not true, it's an internal error.

```cpp
assert((PrescriptivenessModifier == (PrescriptivenessModifierLoc.isValid() ? OMPC_NUMTHREADS_strict : OMPC_NUMTHREADS_unknown)) && 
       "Unexpected num_threads prescriptiveness modifier.");
assert((DimsModifier == (DimsModifierLoc.isValid() ? OMPC_NUMTHREADS_dims : OMPC_NUMTHREADS_unknown)) && 
       "Unexpected num_threads dims modifier.");
```

Are you OK with adding the asserts?

https://github.com/llvm/llvm-project/pull/208353