Skip to content

Commit 68e15af

Browse files
kparzyszgithub-actions[bot]
kparzysz
authored and
github-actions[bot]
committed
Automerge: [flang][OpenMP] Reject blank common blocks more gracefully (#159626)
Parse them as "invalid" OmpObjects, then emit a diagnostic in semantic checks.
2 parents 0d72aac + d89de09 commit 68e15af

File tree

11 files changed

+132
-58
lines changed

11 files changed

+132
-58
lines changed

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -633,6 +633,8 @@ class ParseTreeDumper {
633633
NODE(parser, OmpNumTasksClause)
634634
NODE(OmpNumTasksClause, Modifier)
635635
NODE(parser, OmpObject)
636+
NODE(OmpObject, Invalid)
637+
NODE_ENUM(OmpObject::Invalid, Kind)
636638
NODE(parser, OmpObjectList)
637639
NODE(parser, OmpOrderClause)
638640
NODE(OmpOrderClause, Modifier)

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3505,8 +3505,15 @@ struct OmpDirectiveName {
35053505
// in slashes). An extended list item is a list item or a procedure Name.
35063506
// variable-name | / common-block / | array-sections
35073507
struct OmpObject {
3508+
// Blank common blocks are not valid objects. Parse them to emit meaningful
3509+
// diagnostics.
3510+
struct Invalid {
3511+
ENUM_CLASS(Kind, BlankCommonBlock);
3512+
WRAPPER_CLASS_BOILERPLATE(Invalid, Kind);
3513+
CharBlock source;
3514+
};
35083515
UNION_CLASS_BOILERPLATE(OmpObject);
3509-
std::variant<Designator, /*common block*/ Name> u;
3516+
std::variant<Designator, /*common block*/ Name, Invalid> u;
35103517
};
35113518

35123519
WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);

‎flang/lib/Parser/openmp-parsers.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1024,8 +1024,11 @@ TYPE_PARSER(construct<OmpNumTasksClause>(
10241024
maybe(nonemptyList(Parser<OmpNumTasksClause::Modifier>{}) / ":"),
10251025
scalarIntExpr))
10261026

1027-
TYPE_PARSER(
1028-
construct<OmpObject>(designator) || "/" >> construct<OmpObject>(name) / "/")
1027+
TYPE_PARSER( //
1028+
construct<OmpObject>(designator) ||
1029+
"/" >> construct<OmpObject>(name) / "/" ||
1030+
construct<OmpObject>(sourced(construct<OmpObject::Invalid>(
1031+
"//"_tok >> pure(OmpObject::Invalid::Kind::BlankCommonBlock)))))
10291032

10301033
// OMP 5.0 2.19.4.5 LASTPRIVATE ([lastprivate-modifier :] list)
10311034
TYPE_PARSER(construct<OmpLastprivateClause>(

‎flang/lib/Parser/unparse.cpp‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2168,10 +2168,22 @@ class UnparseVisitor {
21682168
void Unparse(const OmpContextSelectorSpecification &x) { Walk(x.v, ", "); }
21692169

21702170
void Unparse(const OmpObject &x) {
2171-
common::visit(common::visitors{
2172-
[&](const Designator &y) { Walk(y); },
2173-
[&](const Name &y) { Put("/"), Walk(y), Put("/"); },
2174-
},
2171+
common::visit( //
2172+
common::visitors{
2173+
[&](const Designator &y) { Walk(y); },
2174+
[&](const Name &y) {
2175+
Put("/");
2176+
Walk(y);
2177+
Put("/");
2178+
},
2179+
[&](const OmpObject::Invalid &y) {
2180+
switch (y.v) {
2181+
case OmpObject::Invalid::Kind::BlankCommonBlock:
2182+
Put("//");
2183+
break;
2184+
}
2185+
},
2186+
},
21752187
x.u);
21762188
}
21772189
void Unparse(const OmpDirectiveNameModifier &x) {

‎flang/lib/Semantics/check-omp-loop.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,10 @@ void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
491491
checkReductionSymbolInScan(name);
492492
}
493493
},
494-
[&](const auto &name) { checkReductionSymbolInScan(&name); },
494+
[&](const parser::Name &name) {
495+
checkReductionSymbolInScan(&name);
496+
},
497+
[&](const parser::OmpObject::Invalid &invalid) {},
495498
},
496499
ompObj.u);
497500
}

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

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,8 @@ bool OmpStructureChecker::CheckAllowedClause(llvmOmpClause clause) {
269269
}
270270

271271
void OmpStructureChecker::AnalyzeObject(const parser::OmpObject &object) {
272-
if (std::holds_alternative<parser::Name>(object.u)) {
272+
if (std::holds_alternative<parser::Name>(object.u) ||
273+
std::holds_alternative<parser::OmpObject::Invalid>(object.u)) {
273274
// Do not analyze common block names. The analyzer will flag an error
274275
// on those.
275276
return;
@@ -294,7 +295,12 @@ void OmpStructureChecker::AnalyzeObject(const parser::OmpObject &object) {
294295
}
295296
evaluate::ExpressionAnalyzer ea{context_};
296297
auto restore{ea.AllowWholeAssumedSizeArray(true)};
297-
common::visit([&](auto &&s) { ea.Analyze(s); }, object.u);
298+
common::visit( //
299+
common::visitors{
300+
[&](auto &&s) { ea.Analyze(s); },
301+
[&](const parser::OmpObject::Invalid &invalid) {},
302+
},
303+
object.u);
298304
}
299305

300306
void OmpStructureChecker::AnalyzeObjects(const parser::OmpObjectList &objects) {
@@ -538,6 +544,7 @@ void OmpStructureChecker::CheckPredefinedAllocatorRestriction(
538544
[&](const parser::Name &name) {
539545
CheckPredefinedAllocatorRestriction(source, name);
540546
},
547+
[&](const parser::OmpObject::Invalid &invalid) {},
541548
},
542549
ompObject.u);
543550
}
@@ -1290,7 +1297,11 @@ void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar(
12901297
void OmpStructureChecker::CheckThreadprivateOrDeclareTargetVar(
12911298
const parser::OmpObjectList &objList) {
12921299
for (const auto &ompObject : objList.v) {
1293-
common::visit([&](auto &&s) { CheckThreadprivateOrDeclareTargetVar(s); },
1300+
common::visit( //
1301+
common::visitors{
1302+
[&](auto &&s) { CheckThreadprivateOrDeclareTargetVar(s); },
1303+
[&](const parser::OmpObject::Invalid &invalid) {},
1304+
},
12941305
ompObject.u);
12951306
}
12961307
}
@@ -1422,8 +1433,14 @@ void OmpStructureChecker::Enter(const parser::OpenMPDepobjConstruct &x) {
14221433
// refer to the same depend object as the depobj argument of the construct.
14231434
if (clause.Id() == llvm::omp::Clause::OMPC_destroy) {
14241435
auto getObjSymbol{[&](const parser::OmpObject &obj) {
1425-
return common::visit(
1426-
[&](auto &&s) { return GetLastName(s).symbol; }, obj.u);
1436+
return common::visit( //
1437+
common::visitors{
1438+
[&](auto &&s) { return GetLastName(s).symbol; },
1439+
[&](const parser::OmpObject::Invalid &invalid) {
1440+
return static_cast<Symbol *>(nullptr);
1441+
},
1442+
},
1443+
obj.u);
14271444
}};
14281445
auto getArgSymbol{[&](const parser::OmpArgument &arg) {
14291446
if (auto *locator{std::get_if<parser::OmpLocator>(&arg.u)}) {
@@ -1438,9 +1455,7 @@ void OmpStructureChecker::Enter(const parser::OpenMPDepobjConstruct &x) {
14381455
if (const std::optional<parser::OmpDestroyClause> &destroy{wrapper.v}) {
14391456
const Symbol *constrSym{getArgSymbol(arguments.v.front())};
14401457
const Symbol *clauseSym{getObjSymbol(destroy->v)};
1441-
assert(constrSym && "Unresolved depobj construct symbol");
1442-
assert(clauseSym && "Unresolved destroy symbol on depobj construct");
1443-
if (constrSym != clauseSym) {
1458+
if (constrSym && clauseSym && constrSym != clauseSym) {
14441459
context_.Say(x.source,
14451460
"The DESTROY clause must refer to the same object as the "
14461461
"DEPOBJ construct"_err_en_US);
@@ -1678,6 +1693,7 @@ void OmpStructureChecker::CheckSymbolNames(
16781693
ContextDirectiveAsFortran());
16791694
}
16801695
},
1696+
[&](const parser::OmpObject::Invalid &invalid) {},
16811697
},
16821698
ompObject.u);
16831699
}
@@ -2698,6 +2714,7 @@ void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
26982714
}
26992715
}
27002716
},
2717+
[&](const parser::OmpObject::Invalid &invalid) {},
27012718
},
27022719
ompObject.u);
27032720
}
@@ -3405,6 +3422,7 @@ void OmpStructureChecker::CheckVarIsNotPartOfAnotherVar(
34053422
}
34063423
},
34073424
[&](const parser::Name &name) {},
3425+
[&](const parser::OmpObject::Invalid &invalid) {},
34083426
},
34093427
ompObject.u);
34103428
}
@@ -4090,11 +4108,11 @@ void OmpStructureChecker::CheckStructureComponent(
40904108
}};
40914109

40924110
for (const auto &object : objects.v) {
4093-
common::visit(
4094-
common::visitors{
4095-
CheckComponent,
4096-
[&](const parser::Name &name) {},
4097-
},
4111+
common::visit(common::visitors{
4112+
CheckComponent,
4113+
[&](const parser::Name &name) {},
4114+
[&](const parser::OmpObject::Invalid &invalid) {},
4115+
},
40984116
object.u);
40994117
}
41004118
}

‎flang/lib/Semantics/openmp-utils.cpp‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ struct ContiguousHelper {
225225
std::optional<bool> IsContiguous(
226226
SemanticsContext &semaCtx, const parser::OmpObject &object) {
227227
return common::visit( //
228-
common::visitors{
228+
common::visitors{//
229229
[&](const parser::Name &x) {
230230
// Any member of a common block must be contiguous.
231231
return std::optional<bool>{true};
@@ -237,7 +237,9 @@ std::optional<bool> IsContiguous(
237237
}
238238
return std::optional<bool>{};
239239
},
240-
},
240+
[&](const parser::OmpObject::Invalid &) {
241+
return std::optional<bool>{};
242+
}},
241243
object.u);
242244
}
243245

‎flang/lib/Semantics/resolve-directives.cpp‎

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3121,14 +3121,24 @@ void OmpAttributeVisitor::ResolveOmpCommonBlock(
31213121

31223122
void OmpAttributeVisitor::ResolveOmpObject(
31233123
const parser::OmpObject &ompObject, Symbol::Flag ompFlag) {
3124-
common::visit(common::visitors{
3125-
[&](const parser::Designator &designator) {
3126-
ResolveOmpDesignator(designator, ompFlag);
3127-
},
3128-
[&](const parser::Name &name) { // common block
3129-
ResolveOmpCommonBlock(name, ompFlag);
3130-
},
3131-
},
3124+
common::visit( //
3125+
common::visitors{
3126+
[&](const parser::Designator &designator) {
3127+
ResolveOmpDesignator(designator, ompFlag);
3128+
},
3129+
[&](const parser::Name &name) { // common block
3130+
ResolveOmpCommonBlock(name, ompFlag);
3131+
},
3132+
[&](const parser::OmpObject::Invalid &invalid) {
3133+
switch (invalid.v) {
3134+
SWITCH_COVERS_ALL_CASES
3135+
case parser::OmpObject::Invalid::Kind::BlankCommonBlock:
3136+
context_.Say(invalid.source,
3137+
"Blank common blocks are not allowed as directive or clause arguments"_err_en_US);
3138+
break;
3139+
}
3140+
},
3141+
},
31323142
ompObject.u);
31333143
}
31343144

‎flang/lib/Semantics/resolve-names.cpp‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1625,25 +1625,33 @@ class OmpVisitor : public virtual DeclarationVisitor {
16251625
void Post(const parser::OpenMPThreadprivate &) { SkipImplicitTyping(false); }
16261626
bool Pre(const parser::OpenMPDeclareTargetConstruct &x) {
16271627
const auto &spec{std::get<parser::OmpDeclareTargetSpecifier>(x.t)};
1628-
auto populateDeclareTargetNames{
1629-
[this](const parser::OmpObjectList &objectList) {
1630-
for (const auto &ompObject : objectList.v) {
1631-
common::visit(
1632-
common::visitors{
1633-
[&](const parser::Designator &designator) {
1634-
if (const auto *name{
1635-
semantics::getDesignatorNameIfDataRef(
1636-
designator)}) {
1637-
specPartState_.declareTargetNames.insert(name->source);
1638-
}
1639-
},
1640-
[&](const parser::Name &name) {
1641-
specPartState_.declareTargetNames.insert(name.source);
1642-
},
1628+
auto populateDeclareTargetNames{[this](const parser::OmpObjectList
1629+
&objectList) {
1630+
for (const auto &ompObject : objectList.v) {
1631+
common::visit(
1632+
common::visitors{
1633+
[&](const parser::Designator &designator) {
1634+
if (const auto *name{
1635+
semantics::getDesignatorNameIfDataRef(designator)}) {
1636+
specPartState_.declareTargetNames.insert(name->source);
1637+
}
16431638
},
1644-
ompObject.u);
1645-
}
1646-
}};
1639+
[&](const parser::Name &name) {
1640+
specPartState_.declareTargetNames.insert(name.source);
1641+
},
1642+
[&](const parser::OmpObject::Invalid &invalid) {
1643+
switch (invalid.v) {
1644+
SWITCH_COVERS_ALL_CASES
1645+
case parser::OmpObject::Invalid::Kind::BlankCommonBlock:
1646+
context().Say(invalid.source,
1647+
"Blank common blocks are not allowed as directive or clause arguments"_err_en_US);
1648+
break;
1649+
}
1650+
},
1651+
},
1652+
ompObject.u);
1653+
}
1654+
}};
16471655

16481656
if (const auto *objectList{parser::Unwrap<parser::OmpObjectList>(spec.u)}) {
16491657
populateDeclareTargetNames(*objectList);

‎flang/test/Parser/OpenMP/threadprivate-blank-common-block.f90‎

Lines changed: 0 additions & 9 deletions
This file was deleted.

0 commit comments

Comments
 (0)