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

[via cfe-commits]

Author: Erich Keane
Date: 2026-04-13T16:47:21-07:00
New Revision: a6a2a717c57aaa3fd3beb98bfa55c2f2f285a024

URL: https://github.com/llvm/llvm-project/commit/a6a2a717c57aaa3fd3beb98bfa55c2f2f285a024
DIFF: https://github.com/llvm/llvm-project/commit/a6a2a717c57aaa3fd3beb98bfa55c2f2f285a024.diff

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

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

Added: 
    

Modified: 
    clang/lib/Sema/TreeTransform.h
    clang/test/SemaOpenACC/loop-construct-collapse-clause.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 07499ff2cd392..40187f71231bd 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -12651,13 +12651,26 @@ 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());
 
+  // 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;
+
   ParsedClause.setCollapseDetails(C.hasForce(), NewLoopCount.get());
   NewClause = OpenACCCollapseClause::Create(
       Self.getSema().getASTContext(), ParsedClause.getBeginLoc(),
@@ -12681,6 +12694,11 @@ 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;
+
     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>();
+}
+}