[clang] [OpenACC] Fix Crash on collapse that doesn't check its transform (PR #191836)

[Erich Keane via cfe-commits]

https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/191836

>From dea2d37d5bfab3de13c9c9a9da4fcf942fb0a038 Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Mon, 13 Apr 2026 08:14:43 -0700
Subject: [PATCH 1/2] [OpenACC] Fix Crash on collapse that doesn't check its
 transform

GH191833 reports a problem with tree transformation of a collapse
clause when the expression in the clause is invalid. This patch makes
sure we skip out of transforming this clause if it ever encounters an
invalid expression.

I also analyzed the rest of the clauses in this visitor and found 1
other that was suspicious, so I added a check for that one as well.  The
rest seemingly were all done correctly.

Fixes: #191833
---
 clang/lib/Sema/TreeTransform.h                 | 12 ++++++++++++
 .../loop-construct-collapse-clause.cpp         | 18 ++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 07499ff2cd392..9aa5396da4e00 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12651,13 +12651,22 @@ void OpenACCClauseTransform<Derived>::VisitCollapseClause(
 
   ExprResult NewLoopCount = Self.TransformExpr(LoopCount);
 
+  if (!NewLoopCount.isUsable())
+    return;
+
   NewLoopCount = Self.getSema().OpenACC().ActOnIntExpr(
       OpenACCDirectiveKind::Invalid, ParsedClause.getClauseKind(),
       NewLoopCount.get()->getBeginLoc(), NewLoopCount.get());
 
+  if (!NewLoopCount.isUsable())
+    return;
+
   NewLoopCount =
       Self.getSema().OpenACC().CheckCollapseLoopCount(NewLoopCount.get());
 
+  if (!NewLoopCount.isUsable())
+    return;
+
   ParsedClause.setCollapseDetails(C.hasForce(), NewLoopCount.get());
   NewClause = OpenACCCollapseClause::Create(
       Self.getSema().getASTContext(), ParsedClause.getBeginLoc(),
@@ -12681,6 +12690,9 @@ void OpenACCClauseTransform<Derived>::VisitTileClause(
         OpenACCDirectiveKind::Invalid, ParsedClause.getClauseKind(),
         NewSizeExpr.get()->getBeginLoc(), NewSizeExpr.get());
 
+    if (!NewSizeExpr.isUsable())
+      return;
+
     NewSizeExpr = Self.getSema().OpenACC().CheckTileSizeExpr(NewSizeExpr.get());
 
     if (!NewSizeExpr.isUsable())
diff --git a/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp b/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp
index ed71552dfdb10..adc481837f74a 100644
--- a/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp
+++ b/clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp
@@ -594,3 +594,21 @@ void no_dupes_since_last_device_type() {
 #pragma acc loop collapse(NotConstexpr) device_type(radeon, nvidia) collapse(NotConstexpr) device_type(host) collapse(NotConstexpr)
   for(unsigned j = 0; j < 5; ++j);
 }
+
+namespace gh191833 {
+  // Instantiating a collapse referring to an invalid struct caused us to try
+  // get the begin-loc of the `T{}` statement when we shouldn't.
+struct S {
+  typename T; // expected-error{{expected a qualified name after 'typename'}}
+};
+
+template<typename T>
+void foo() {
+#pragma acc loop collapse(T{})
+  for (int i = 0; i < 5; ++i);
+}
+
+void bar() {
+  foo<S>();
+}
+}

>From dcdb794ffcdef07a29a65368d48a24c5091a0120 Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Mon, 13 Apr 2026 12:04:48 -0700
Subject: [PATCH 2/2] add FIXME comments as requested in review

---
 clang/lib/Sema/TreeTransform.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 9aa5396da4e00..40187f71231bd 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12658,12 +12658,16 @@ void OpenACCClauseTransform<Derived>::VisitCollapseClause(
       OpenACCDirectiveKind::Invalid, ParsedClause.getClauseKind(),
       NewLoopCount.get()->getBeginLoc(), NewLoopCount.get());
 
+  // FIXME: It isn't clear whether this is properly tested here, we should
+  // probably see if we can come up with a test for this.
   if (!NewLoopCount.isUsable())
     return;
 
   NewLoopCount =
       Self.getSema().OpenACC().CheckCollapseLoopCount(NewLoopCount.get());
 
+  // FIXME: It isn't clear whether this is properly tested here, we should
+  // probably see if we can come up with a test for this.
   if (!NewLoopCount.isUsable())
     return;
 
@@ -12690,6 +12694,8 @@ void OpenACCClauseTransform<Derived>::VisitTileClause(
         OpenACCDirectiveKind::Invalid, ParsedClause.getClauseKind(),
         NewSizeExpr.get()->getBeginLoc(), NewSizeExpr.get());
 
+    // FIXME: It isn't clear whether this is properly tested here, we should
+    // probably see if we can come up with a test for this.
     if (!NewSizeExpr.isUsable())
       return;