[flang] Improve OpenACC SELF clause parser #135883
Conversation
klausler
requested review from
clementval,
akuhlens,
erichkeane and
eugeneepshteyn
llvmbot
added
flang
|
@llvm/pr-subscribers-openacc @llvm/pr-subscribers-flang-semantics Author: Peter Klausler (klausler) ChangesThe current parser can fail on "self(x * 2)" by recognizing just "x" as a one-element list of object names and then failing at a higher level because it never reached the right parenthesis. Add lookahead checks and error recovery. Fixes #135810. Full diff: https://github.com/llvm/llvm-project/pull/135883.diff 3 Files Affected:
diff --git a/flang/lib/Parser/openacc-parsers.cpp b/flang/lib/Parser/openacc-parsers.cpp
index fb731ee52cbba..5e0a56b1d374c 100644
--- a/flang/lib/Parser/openacc-parsers.cpp
+++ b/flang/lib/Parser/openacc-parsers.cpp
@@ -126,8 +126,13 @@ TYPE_PARSER(construct<AccDefaultClause>(
// SELF clause is either a simple optional condition for compute construct
// or a synonym of the HOST clause for the update directive 2.14.4 holding
// an object list.
-TYPE_PARSER(construct<AccSelfClause>(Parser<AccObjectList>{}) ||
- construct<AccSelfClause>(scalarLogicalExpr))
+TYPE_PARSER(
+ construct<AccSelfClause>(Parser<AccObjectList>{}) / lookAhead(")"_tok) ||
+ construct<AccSelfClause>(scalarLogicalExpr / lookAhead(")"_tok)) ||
+ construct<AccSelfClause>(
+ recovery(fail<std::optional<ScalarLogicalExpr>>(
+ "logical expression or object list expected"_err_en_US),
+ SkipTo<')'>{} >> pure<std::optional<ScalarLogicalExpr>>())))
// Modifier for copyin, copyout, cache and create
TYPE_PARSER(construct<AccDataModifier>(
diff --git a/flang/test/Semantics/OpenACC/bug135810-1.f90 b/flang/test/Semantics/OpenACC/bug135810-1.f90
new file mode 100644
index 0000000000000..c40f4720b4f06
--- /dev/null
+++ b/flang/test/Semantics/OpenACC/bug135810-1.f90
@@ -0,0 +1,9 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenacc
+integer function square(x)
+ implicit none
+ integer, intent(in) :: x
+!ERROR: Must have LOGICAL type, but is INTEGER(4)
+ !$acc parallel self(x * 2)
+ !$acc end parallel
+ square = x * x
+end function square
diff --git a/flang/test/Semantics/OpenACC/bug135810-2.f90 b/flang/test/Semantics/OpenACC/bug135810-2.f90
new file mode 100644
index 0000000000000..8ed608b0b4cc9
--- /dev/null
+++ b/flang/test/Semantics/OpenACC/bug135810-2.f90
@@ -0,0 +1,12 @@
+! RUN: %python %S/../test_errors.py %s %flang -fopenacc
+integer function square(x)
+ implicit none
+ integer, intent(in) :: x
+!ERROR: logical expression or object list expected
+ !$acc parallel self(,)
+ !$acc end parallel
+!ERROR: logical expression or object list expected
+ !$acc parallel self(.true., )
+ !$acc end parallel
+ square = x * x
+end function square
|
| ! RUN: %python %S/../test_errors.py %s %flang -fopenacc | ||
| integer function square(x) | ||
| implicit none | ||
| integer, intent(in) :: x |
There was a problem hiding this comment.
How about also adding a positive test for something like !$acc parallel self(x * 2 .eq. 4) ?
There was a problem hiding this comment.
Generally looks fine to me, but my experience with this codebase is only a few hours old. @razvanlupusoru had some additional knowledge here, and might wish to chime in.
| !ERROR: logical expression or object list expected | ||
| !$acc parallel self(,) | ||
| !$acc end parallel | ||
| !ERROR: logical expression or object list expected |
There was a problem hiding this comment.
Do you have a test that !$acc parallel self(object-list) would cause a diagnostic? (as self only takes an object list when used on an update construct).
There was a problem hiding this comment.
That's done by semantics, not the parser; but I'll add a test here since there isn't one elsewhere.
The current parser can fail on "self(x * 2)" by recognizing just "x" as a one-element list of object names and then failing at a higher level because it never reached the right parenthesis. Add lookahead checks and error recovery. Fixes llvm#135810.
The current parser can fail on "self(x * 2)" by recognizing just "x" as a one-element list of object names and then failing at a higher level because it never reached the right parenthesis. Add lookahead checks and error recovery.
Fixes #135810.