Skip to content

Commit f84070b

Browse files
committed
[flang][OpenMP] move omp end sections validation to semantics
See #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 16662ce commit f84070b

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 wihtin 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
@@ -3958,9 +3958,12 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
39583958
List<Clause> clauses = makeClauses(
39593959
std::get<parser::OmpClauseList>(beginSectionsDirective.t), semaCtx);
39603960
const auto &endSectionsDirective =
3961-
std::get<parser::OmpEndSectionsDirective>(sectionsConstruct.t);
3961+
std::get<std::optional<parser::OmpEndSectionsDirective>>(
3962+
sectionsConstruct.t);
3963+
assert(endSectionsDirective &&
3964+
"Missing end section directive should have been handled in semantics");
39623965
clauses.append(makeClauses(
3963-
std::get<parser::OmpClauseList>(endSectionsDirective.t), semaCtx));
3966+
std::get<parser::OmpClauseList>(endSectionsDirective->t), semaCtx));
39643967
mlir::Location currentLocation = converter.getCurrentLocation();
39653968

39663969
llvm::omp::Directive directive =

flang/lib/Parser/openmp-parsers.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1917,7 +1917,7 @@ TYPE_PARSER(sourced(construct<OpenMPSectionsConstruct>(
19171917
construct<OpenMPSectionConstruct>(maybe(sectionDir), block))),
19181918
many(construct<OpenMPConstruct>(
19191919
sourced(construct<OpenMPSectionConstruct>(sectionDir, block))))),
1920-
Parser<OmpEndSectionsDirective>{} / endOmpLine)))
1920+
maybe(Parser<OmpEndSectionsDirective>{} / endOmpLine))))
19211921

19221922
static bool IsExecutionPart(const OmpDirectiveName &name) {
19231923
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
@@ -1063,14 +1063,23 @@ void OmpStructureChecker::Leave(const parser::OmpBeginDirective &) {
10631063
void OmpStructureChecker::Enter(const parser::OpenMPSectionsConstruct &x) {
10641064
const auto &beginSectionsDir{
10651065
std::get<parser::OmpBeginSectionsDirective>(x.t)};
1066-
const auto &endSectionsDir{std::get<parser::OmpEndSectionsDirective>(x.t)};
1066+
const auto &endSectionsDir{
1067+
std::get<std::optional<parser::OmpEndSectionsDirective>>(x.t)};
10671068
const auto &beginDir{
10681069
std::get<parser::OmpSectionsDirective>(beginSectionsDir.t)};
1069-
const auto &endDir{std::get<parser::OmpSectionsDirective>(endSectionsDir.t)};
1070+
PushContextAndClauseSets(beginDir.source, beginDir.v);
1071+
1072+
if (!endSectionsDir) {
1073+
context_.Say(beginSectionsDir.source,
1074+
"Expected OpenMP END SECTIONS directive"_err_en_US);
1075+
// Following code assumes the option is present.
1076+
return;
1077+
}
1078+
1079+
const auto &endDir{std::get<parser::OmpSectionsDirective>(endSectionsDir->t)};
10701080
CheckMatching<parser::OmpSectionsDirective>(beginDir, endDir);
10711081

1072-
PushContextAndClauseSets(beginDir.source, beginDir.v);
1073-
AddEndDirectiveClauses(std::get<parser::OmpClauseList>(endSectionsDir.t));
1082+
AddEndDirectiveClauses(std::get<parser::OmpClauseList>(endSectionsDir->t));
10741083

10751084
const auto &sectionBlocks{std::get<std::list<parser::OpenMPConstruct>>(x.t)};
10761085
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)