[clang] [OpenACC] Fix invalid routine case where 'bind' didn't exist (PR #192270)

[via cfe-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Erich Keane (erichkeane)

<details>
<summary>Changes</summary>

For some reason I'd failed to check the result of `find_if` and just assumed that the `bind` clause must exist!  Looking through my other tests, I've validated every other combination other than this one for some reason.

Fixes: #<!-- -->192245

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


2 Files Affected:

- (modified) clang/lib/Sema/SemaOpenACC.cpp (+31-28) 
- (modified) clang/test/SemaOpenACC/routine-construct-clauses.cpp (+11) 


``````````diff
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
index 30e07d7b145af..b8ffc01c753f7 100644
--- a/clang/lib/Sema/SemaOpenACC.cpp
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -2416,37 +2416,40 @@ void SemaOpenACC::CheckRoutineDecl(SourceLocation DirLoc,
   }
 
   auto BindItr = llvm::find_if(Clauses, llvm::IsaPred<OpenACCBindClause>);
-  for (auto *A : NextParsedFDecl->attrs()) {
-    // OpenACC 3.3 2.15:
-    // If a procedure has a bind clause on both the declaration and definition
-    // than they both must bind to the same name.
-    if (auto *RA = dyn_cast<OpenACCRoutineDeclAttr>(A)) {
-      auto OtherBindItr =
-          llvm::find_if(RA->Clauses, llvm::IsaPred<OpenACCBindClause>);
-      if (OtherBindItr != RA->Clauses.end() &&
-          (*cast<OpenACCBindClause>(*BindItr)) !=
-              (*cast<OpenACCBindClause>(*OtherBindItr))) {
-        Diag((*BindItr)->getBeginLoc(), diag::err_acc_duplicate_unnamed_bind);
-        Diag((*OtherBindItr)->getEndLoc(), diag::note_acc_previous_clause_here)
-            << (*BindItr)->getClauseKind();
-        return;
+      if (BindItr != Clauses.end()) {
+    for (auto *A : NextParsedFDecl->attrs()) {
+      // OpenACC 3.3 2.15:
+      // If a procedure has a bind clause on both the declaration and definition
+      // than they both must bind to the same name.
+      if (auto *RA = dyn_cast<OpenACCRoutineDeclAttr>(A)) {
+        auto OtherBindItr =
+            llvm::find_if(RA->Clauses, llvm::IsaPred<OpenACCBindClause>);
+        if (OtherBindItr != RA->Clauses.end() &&
+            (*cast<OpenACCBindClause>(*BindItr)) !=
+                (*cast<OpenACCBindClause>(*OtherBindItr))) {
+          Diag((*BindItr)->getBeginLoc(), diag::err_acc_duplicate_unnamed_bind);
+          Diag((*OtherBindItr)->getEndLoc(),
+               diag::note_acc_previous_clause_here)
+              << (*BindItr)->getClauseKind();
+          return;
+        }
       }
-    }
 
-    // OpenACC 3.3 2.15:
-    // A bind clause may not bind to a routine name that has a visible bind
-    // clause.
-    // We take the combo of these two 2.15 restrictions to mean that the
-    // 'declaration'/'definition' quote is an exception to this. So we're going
-    // to disallow mixing of the two types entirely.
-    if (auto *RA = dyn_cast<OpenACCRoutineAnnotAttr>(A);
-        RA && RA->getRange().getEnd().isValid()) {
-      Diag((*BindItr)->getBeginLoc(), diag::err_acc_duplicate_bind);
-      Diag(RA->getRange().getEnd(), diag::note_acc_previous_clause_here)
-          << "bind";
-      return;
+      // OpenACC 3.3 2.15:
+      // A bind clause may not bind to a routine name that has a visible bind
+      // clause.
+      // We take the combo of these two 2.15 restrictions to mean that the
+      // 'declaration'/'definition' quote is an exception to this. So we're
+      // going to disallow mixing of the two types entirely.
+      if (auto *RA = dyn_cast<OpenACCRoutineAnnotAttr>(A);
+          RA && RA->getRange().getEnd().isValid()) {
+        Diag((*BindItr)->getBeginLoc(), diag::err_acc_duplicate_bind);
+        Diag(RA->getRange().getEnd(), diag::note_acc_previous_clause_here)
+            << "bind";
+        return;
+      }
     }
-  }
+      }
 
   CreateRoutineDeclAttr(*this, DirLoc, Clauses, NextParsedFDecl);
 }
diff --git a/clang/test/SemaOpenACC/routine-construct-clauses.cpp b/clang/test/SemaOpenACC/routine-construct-clauses.cpp
index 91f233537e4ca..4c7152861afc1 100644
--- a/clang/test/SemaOpenACC/routine-construct-clauses.cpp
+++ b/clang/test/SemaOpenACC/routine-construct-clauses.cpp
@@ -803,3 +803,14 @@ namespace OtherDupes {
 #pragma acc routine device_type(nvidia) vector device_type(acc_device_nvidia)
   void Func3();
 }
+
+namespace GH192245 {
+  // These are fine, but we were assuming a bind-clause during checking that
+  // didnt' exist in the Func1 case.
+#pragma acc routine seq
+#pragma acc routine seq bind("asdf")
+  void Func1();
+#pragma acc routine seq bind("asdf")
+#pragma acc routine seq
+  void Func2();
+}

``````````

</details>


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