Skip to content

Commit 044e1aa

Browse files
authored
[flang][OpenMP] move omp end sections validation to semantics (llvm#154740)
See llvm#90452. The old parse tree errors exploded to thousands of unhelpful lines when there were multiple missing end directives. Instead, allow a missing end directive in the parse tree then validate that it is present during semantics (where the error messages are a lot easier to control).
1 parent 3c2df33 commit 044e1aa

File tree

6 files changed

+28
-9
lines changed

6 files changed

+28
-9
lines changed

flang/include/flang/Parser/parse-tree.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4893,8 +4893,11 @@ struct OpenMPSectionsConstruct {
48934893
CharBlock source;
48944894
// Each of the OpenMPConstructs in the list below contains an
48954895
// OpenMPSectionConstruct. This is guaranteed by the parser.
4896+
// The end sections directive is optional here because it is difficult to
4897+
// generate helpful error messages for a missing end directive within the
4898+
// parser. Semantics will generate an error if this is absent.
48964899
std::tuple<OmpBeginSectionsDirective, std::list<OpenMPConstruct>,
4897-
OmpEndSectionsDirective>
4900+
std::optional<OmpEndSectionsDirective>>
48984901
t;
48994902
};
49004903

flang/lib/Lower/OpenMP/OpenMP.cpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3979,9 +3979,12 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
39793979
List<Clause> clauses = makeClauses(
39803980
std::get<parser::OmpClauseList>(beginSectionsDirective.t), semaCtx);
39813981
const auto &endSectionsDirective =
3982-
std::get<parser::OmpEndSectionsDirective>(sectionsConstruct.t);
3982+
std::get<std::optional<parser::OmpEndSectionsDirective>>(
3983+
sectionsConstruct.t);
3984+
assert(endSectionsDirective &&
3985+
"Missing end section directive should have been handled in semantics");
39833986
clauses.append(makeClauses(
3984-
std::get<parser::OmpClauseList>(endSectionsDirective.t), semaCtx));
3987+
std::get<parser::OmpClauseList>(endSectionsDirective->t), semaCtx));
39853988
mlir::Location currentLocation = converter.getCurrentLocation();
39863989

39873990
llvm::omp::Directive directive =

flang/lib/Parser/openmp-parsers.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1921,7 +1921,7 @@ TYPE_PARSER(sourced(construct<OpenMPSectionsConstruct>(
19211921
construct<OpenMPSectionConstruct>(maybe(sectionDir), block))),
19221922
many(construct<OpenMPConstruct>(
19231923
sourced(construct<OpenMPSectionConstruct>(sectionDir, block))))),
1924-
Parser<OmpEndSectionsDirective>{} / endOmpLine)))
1924+
maybe(Parser<OmpEndSectionsDirective>{} / endOmpLine))))
19251925

19261926
static bool IsExecutionPart(const OmpDirectiveName &name) {
19271927
return name.IsExecutionPart();

flang/lib/Parser/unparse.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2788,7 +2788,7 @@ class UnparseVisitor {
27882788
Walk(std::get<std::list<OpenMPConstruct>>(x.t), "");
27892789
BeginOpenMP();
27902790
Word("!$OMP END ");
2791-
Walk(std::get<OmpEndSectionsDirective>(x.t));
2791+
Walk(std::get<std::optional<OmpEndSectionsDirective>>(x.t));
27922792
Put("\n");
27932793
EndOpenMP();
27942794
}

flang/lib/Semantics/check-omp-structure.cpp

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1138,14 +1138,23 @@ void OmpStructureChecker::Leave(const parser::OmpBeginDirective &) {
11381138
void OmpStructureChecker::Enter(const parser::OpenMPSectionsConstruct &x) {
11391139
const auto &beginSectionsDir{
11401140
std::get<parser::OmpBeginSectionsDirective>(x.t)};
1141-
const auto &endSectionsDir{std::get<parser::OmpEndSectionsDirective>(x.t)};
1141+
const auto &endSectionsDir{
1142+
std::get<std::optional<parser::OmpEndSectionsDirective>>(x.t)};
11421143
const auto &beginDir{
11431144
std::get<parser::OmpSectionsDirective>(beginSectionsDir.t)};
1144-
const auto &endDir{std::get<parser::OmpSectionsDirective>(endSectionsDir.t)};
1145+
PushContextAndClauseSets(beginDir.source, beginDir.v);
1146+
1147+
if (!endSectionsDir) {
1148+
context_.Say(beginSectionsDir.source,
1149+
"Expected OpenMP END SECTIONS directive"_err_en_US);
1150+
// Following code assumes the option is present.
1151+
return;
1152+
}
1153+
1154+
const auto &endDir{std::get<parser::OmpSectionsDirective>(endSectionsDir->t)};
11451155
CheckMatching<parser::OmpSectionsDirective>(beginDir, endDir);
11461156

1147-
PushContextAndClauseSets(beginDir.source, beginDir.v);
1148-
AddEndDirectiveClauses(std::get<parser::OmpClauseList>(endSectionsDir.t));
1157+
AddEndDirectiveClauses(std::get<parser::OmpClauseList>(endSectionsDir->t));
11491158

11501159
const auto &sectionBlocks{std::get<std::list<parser::OpenMPConstruct>>(x.t)};
11511160
for (const parser::OpenMPConstruct &construct : sectionBlocks) {

flang/test/Semantics/OpenMP/missing-end-directive.f90

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,12 @@
66
!$omp parallel
77
! ERROR: Expected OpenMP end directive
88
!$omp task
9+
! ERROR: Expected OpenMP END SECTIONS directive
10+
!$omp sections
911
! ERROR: Expected OpenMP end directive
1012
!$omp parallel
1113
! ERROR: Expected OpenMP end directive
1214
!$omp task
15+
! ERROR: Expected OpenMP END SECTIONS directive
16+
!$omp sections
1317
end

0 commit comments

Comments
 (0)