https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/191836
>From dea2d37d5bfab3de13c9c9a9da4fcf942fb0a038 Mon Sep 17 00:00:00 2001 From: erichkeane <[email protected]> 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 <[email protected]> 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; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
