[flang-commits] [flang] [flang][Semantics][OpenMP] Fix ICE for unknown reduction starting with . (PR #94398)

[Tom Eccles via flang-commits]

https://github.com/tblah updated https://github.com/llvm/llvm-project/pull/94398

>From 8e06b598b8417192645cb6d31a74b871ce1e413c Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Tue, 4 Jun 2024 20:08:32 +0000
Subject: [PATCH 1/2] [flang][Semantics][OpenMP] Fix ICE for unknown reduction
 starting with .

In this case the union inside of the `parser::DefinedOperator` contains a
string name instead of the expected
`parser::DefinedOperator::IntrinsicOperator`. This led to a `std::abort`.

This patch adapts the code so that if it contains a string name we emit
a semantic error.
---
 flang/lib/Semantics/check-omp-structure.cpp | 14 +++++++++++---
 flang/test/Semantics/OpenMP/reduction13.f90 | 10 ++++++++++
 2 files changed, 21 insertions(+), 3 deletions(-)
 create mode 100644 flang/test/Semantics/OpenMP/reduction13.f90

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 5e3a5725c18d2..96beb771eb48e 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2321,9 +2321,17 @@ bool OmpStructureChecker::CheckReductionOperators(
   common::visit(
       common::visitors{
           [&](const parser::DefinedOperator &dOpr) {
-            const auto &intrinsicOp{
-                std::get<parser::DefinedOperator::IntrinsicOperator>(dOpr.u)};
-            ok = CheckIntrinsicOperator(intrinsicOp);
+            const auto *intrinsicOp{
+                std::get_if<parser::DefinedOperator::IntrinsicOperator>(
+                    &dOpr.u)};
+            if (intrinsicOp) {
+              ok = CheckIntrinsicOperator(*intrinsicOp);
+            } else {
+              context_.Say(GetContext().clauseSource,
+                  "Invalid reduction operator in REDUCTION "
+                  "clause."_err_en_US,
+                  ContextDirectiveAsFortran());
+            }
           },
           [&](const parser::ProcedureDesignator &procD) {
             const parser::Name *name{std::get_if<parser::Name>(&procD.u)};
diff --git a/flang/test/Semantics/OpenMP/reduction13.f90 b/flang/test/Semantics/OpenMP/reduction13.f90
new file mode 100644
index 0000000000000..b8d50ff9d26b9
--- /dev/null
+++ b/flang/test/Semantics/OpenMP/reduction13.f90
@@ -0,0 +1,10 @@
+! RUN: %python %S/../test_errors.py %s %flang_fc1 -fopenmp
+! OpenMP Version 4.5
+! 2.15.3.6 Reduction Clause
+program omp_reduction
+  integer :: k
+  ! miss-spelling. Should be "min"
+  !ERROR: Invalid reduction operator in REDUCTION clause.
+  !$omp parallel reduction(.min.:k)
+  !$omp end parallel
+end program omp_reduction

>From fbf9067fff5c0b39b9fb0e122e9790cec99b6528 Mon Sep 17 00:00:00 2001
From: Tom Eccles <tom.eccles at arm.com>
Date: Wed, 5 Jun 2024 09:46:10 +0000
Subject: [PATCH 2/2] Review comments

---
 flang/lib/Semantics/check-omp-structure.cpp | 10 ++++------
 flang/test/Semantics/OpenMP/reduction13.f90 |  2 +-
 2 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 96beb771eb48e..54ce45157537c 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -2321,15 +2321,13 @@ bool OmpStructureChecker::CheckReductionOperators(
   common::visit(
       common::visitors{
           [&](const parser::DefinedOperator &dOpr) {
-            const auto *intrinsicOp{
-                std::get_if<parser::DefinedOperator::IntrinsicOperator>(
-                    &dOpr.u)};
-            if (intrinsicOp) {
+            if (const auto *intrinsicOp{
+                    std::get_if<parser::DefinedOperator::IntrinsicOperator>(
+                        &dOpr.u)}) {
               ok = CheckIntrinsicOperator(*intrinsicOp);
             } else {
               context_.Say(GetContext().clauseSource,
-                  "Invalid reduction operator in REDUCTION "
-                  "clause."_err_en_US,
+                  "Invalid reduction operator in REDUCTION clause."_err_en_US,
                   ContextDirectiveAsFortran());
             }
           },
diff --git a/flang/test/Semantics/OpenMP/reduction13.f90 b/flang/test/Semantics/OpenMP/reduction13.f90
index b8d50ff9d26b9..edb153ce20ef6 100644
--- a/flang/test/Semantics/OpenMP/reduction13.f90
+++ b/flang/test/Semantics/OpenMP/reduction13.f90
@@ -3,7 +3,7 @@
 ! 2.15.3.6 Reduction Clause
 program omp_reduction
   integer :: k
-  ! miss-spelling. Should be "min"
+  ! misspelling. Should be "min"
   !ERROR: Invalid reduction operator in REDUCTION clause.
   !$omp parallel reduction(.min.:k)
   !$omp end parallel