[flang] Acknowledge non-enforcement of C7108 #139169
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-semantics Author: Peter Klausler (klausler) ChangesFortran 2023 constraint C7108 prohibits the use of a structure constructor in a way that is ambiguous with a generic function reference (intrinsic or user-defined). Sadly, no Fortran compiler implements this constraint, and the common portable interpretation seems to be the generic resolution, not the structure constructor. Restructure the processing of structure constructors in expression analysis so that it can be driven both from the parse tree as well as from generic resolution, and then use it to detect ambigous structure constructor / generic function cases, so that a portability warning can be issued. And document this as a new intentional violation of the standard in Extensions.md. Fixes #138807. Patch is 32.42 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/139169.diff 10 Files Affected:
diff --git a/flang/docs/Extensions.md b/flang/docs/Extensions.md
index 5c7751763eab1..00a7e2bac84e6 100644
--- a/flang/docs/Extensions.md
+++ b/flang/docs/Extensions.md
@@ -159,6 +159,11 @@ end
to be constant will generate a compilation error. `ieee_support_standard`
depends in part on `ieee_support_halting`, so this also applies to
`ieee_support_standard` calls.
+* F'2023 constraint C7108 prohibits the use of a structure constructor
+ that could also be interpreted as a generic function reference.
+ No other Fortran compiler enforces C7108 (to our knowledge);
+ they all resolve the ambiguity by interpreting the call as a function
+ reference. We do the same, with a portability warning.
## Extensions, deletions, and legacy features supported by default
diff --git a/flang/include/flang/Semantics/expression.h b/flang/include/flang/Semantics/expression.h
index eee23dba4831f..30f5dfd8a44cd 100644
--- a/flang/include/flang/Semantics/expression.h
+++ b/flang/include/flang/Semantics/expression.h
@@ -394,6 +394,19 @@ class ExpressionAnalyzer {
MaybeExpr AnalyzeComplex(MaybeExpr &&re, MaybeExpr &&im, const char *what);
std::optional<Chevrons> AnalyzeChevrons(const parser::CallStmt &);
+ // CheckStructureConstructor() is used for parsed structure constructors
+ // as well as for generic function references.
+ struct ComponentSpec {
+ ComponentSpec() = default;
+ ComponentSpec(ComponentSpec &&) = default;
+ parser::CharBlock source, exprSource;
+ bool hasKeyword{false};
+ const Symbol *keywordSymbol{nullptr};
+ MaybeExpr expr;
+ };
+ MaybeExpr CheckStructureConstructor(parser::CharBlock typeName,
+ const semantics::DerivedTypeSpec &, std::list<ComponentSpec> &&);
+
MaybeExpr IterativelyAnalyzeSubexpressions(const parser::Expr &);
semantics::SemanticsContext &context_;
diff --git a/flang/include/flang/Support/Fortran-features.h b/flang/include/flang/Support/Fortran-features.h
index 6cb1bcdb0003f..aa3396c46963c 100644
--- a/flang/include/flang/Support/Fortran-features.h
+++ b/flang/include/flang/Support/Fortran-features.h
@@ -54,7 +54,7 @@ ENUM_CLASS(LanguageFeature, BackslashEscapes, OldDebugLines,
PolymorphicActualAllocatableOrPointerToMonomorphicDummy, RelaxedPureDummy,
UndefinableAsynchronousOrVolatileActual, AutomaticInMainProgram, PrintCptr,
SavedLocalInSpecExpr, PrintNamelist, AssumedRankPassedToNonAssumedRank,
- IgnoreIrrelevantAttributes, Unsigned)
+ IgnoreIrrelevantAttributes, Unsigned, AmbiguousStructureConstructor)
// Portability and suspicious usage warnings
ENUM_CLASS(UsageWarning, Portability, PointerToUndefinable,
diff --git a/flang/lib/Semantics/expression.cpp b/flang/lib/Semantics/expression.cpp
index e139bda7e4950..b4cadcfee05b0 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -2063,23 +2063,9 @@ static MaybeExpr ImplicitConvertTo(const semantics::Symbol &sym,
return std::nullopt;
}
-MaybeExpr ExpressionAnalyzer::Analyze(
- const parser::StructureConstructor &structure) {
- auto &parsedType{std::get<parser::DerivedTypeSpec>(structure.t)};
- parser::Name structureType{std::get<parser::Name>(parsedType.t)};
- parser::CharBlock &typeName{structureType.source};
- if (semantics::Symbol *typeSymbol{structureType.symbol}) {
- if (typeSymbol->has<semantics::DerivedTypeDetails>()) {
- semantics::DerivedTypeSpec dtSpec{typeName, typeSymbol->GetUltimate()};
- if (!CheckIsValidForwardReference(dtSpec)) {
- return std::nullopt;
- }
- }
- }
- if (!parsedType.derivedTypeSpec) {
- return std::nullopt;
- }
- const auto &spec{*parsedType.derivedTypeSpec};
+MaybeExpr ExpressionAnalyzer::CheckStructureConstructor(
+ parser::CharBlock typeName, const semantics::DerivedTypeSpec &spec,
+ std::list<ComponentSpec> &&componentSpecs) {
const Symbol &typeSymbol{spec.typeSymbol()};
if (!spec.scope() || !typeSymbol.has<semantics::DerivedTypeDetails>()) {
return std::nullopt; // error recovery
@@ -2090,10 +2076,10 @@ MaybeExpr ExpressionAnalyzer::Analyze(
const Symbol *parentComponent{typeDetails.GetParentComponent(*spec.scope())};
if (typeSymbol.attrs().test(semantics::Attr::ABSTRACT)) { // C796
- AttachDeclaration(Say(typeName,
- "ABSTRACT derived type '%s' may not be used in a "
- "structure constructor"_err_en_US,
- typeName),
+ AttachDeclaration(
+ Say(typeName,
+ "ABSTRACT derived type '%s' may not be used in a structure constructor"_err_en_US,
+ typeName),
typeSymbol); // C7114
}
@@ -2126,19 +2112,19 @@ MaybeExpr ExpressionAnalyzer::Analyze(
// NULL() can be a valid component
auto restorer{AllowNullPointer()};
- for (const auto &component :
- std::get<std::list<parser::ComponentSpec>>(structure.t)) {
- const parser::Expr &expr{
- std::get<parser::ComponentDataSource>(component.t).v.value()};
- parser::CharBlock source{expr.source};
+ for (ComponentSpec &componentSpec : componentSpecs) {
+ parser::CharBlock source{componentSpec.source};
+ parser::CharBlock exprSource{componentSpec.exprSource};
auto restorer{messages.SetLocation(source)};
- const Symbol *symbol{nullptr};
- MaybeExpr value{Analyze(expr)};
+ const Symbol *symbol{componentSpec.keywordSymbol};
+ MaybeExpr &maybeValue{componentSpec.expr};
+ if (!maybeValue.has_value()) {
+ return std::nullopt;
+ }
+ Expr<SomeType> &value{*maybeValue};
std::optional<DynamicType> valueType{DynamicType::From(value)};
- if (const auto &kw{std::get<std::optional<parser::Keyword>>(component.t)}) {
+ if (componentSpec.hasKeyword) {
anyKeyword = true;
- source = kw->v.source;
- symbol = kw->v.symbol;
if (!symbol) {
// Skip overridden inaccessible parent components in favor of
// their later overrides.
@@ -2190,9 +2176,9 @@ MaybeExpr ExpressionAnalyzer::Analyze(
}
}
if (symbol) {
- const semantics::Scope &innermost{context_.FindScope(expr.source)};
+ const semantics::Scope &innermost{context_.FindScope(exprSource)};
if (auto msg{CheckAccessibleSymbol(innermost, *symbol)}) {
- Say(expr.source, std::move(*msg));
+ Say(exprSource, std::move(*msg));
}
if (checkConflicts) {
auto componentIter{
@@ -2200,8 +2186,7 @@ MaybeExpr ExpressionAnalyzer::Analyze(
if (unavailable.find(symbol->name()) != unavailable.cend()) {
// C797, C798
Say(source,
- "Component '%s' conflicts with another component earlier in "
- "this structure constructor"_err_en_US,
+ "Component '%s' conflicts with another component earlier in this structure constructor"_err_en_US,
symbol->name());
} else if (symbol->test(Symbol::Flag::ParentComp)) {
// Make earlier components unavailable once a whole parent appears.
@@ -2219,143 +2204,136 @@ MaybeExpr ExpressionAnalyzer::Analyze(
}
}
unavailable.insert(symbol->name());
- if (value) {
- if (symbol->has<semantics::TypeParamDetails>()) {
- Say(expr.source,
- "Type parameter '%s' may not appear as a component of a structure constructor"_err_en_US,
- symbol->name());
- }
- if (!(symbol->has<semantics::ProcEntityDetails>() ||
- symbol->has<semantics::ObjectEntityDetails>())) {
- continue; // recovery
- }
- if (IsPointer(*symbol)) { // C7104, C7105, C1594(4)
- semantics::CheckStructConstructorPointerComponent(
- context_, *symbol, *value, innermost);
- result.Add(*symbol, Fold(std::move(*value)));
- continue;
- }
- if (IsNullPointer(&*value)) {
- if (IsAllocatable(*symbol)) {
- if (IsBareNullPointer(&*value)) {
- // NULL() with no arguments allowed by 7.5.10 para 6 for
- // ALLOCATABLE.
- result.Add(*symbol, Expr<SomeType>{NullPointer{}});
- continue;
- }
- if (IsNullObjectPointer(&*value)) {
- AttachDeclaration(
- Warn(common::LanguageFeature::
- NullMoldAllocatableComponentValue,
- expr.source,
- "NULL() with arguments is not standard conforming as the value for allocatable component '%s'"_port_en_US,
- symbol->name()),
- *symbol);
- // proceed to check type & shape
- } else {
- AttachDeclaration(
- Say(expr.source,
- "A NULL procedure pointer may not be used as the value for component '%s'"_err_en_US,
- symbol->name()),
- *symbol);
- continue;
- }
+ if (symbol->has<semantics::TypeParamDetails>()) {
+ Say(exprSource,
+ "Type parameter '%s' may not appear as a component of a structure constructor"_err_en_US,
+ symbol->name());
+ }
+ if (!(symbol->has<semantics::ProcEntityDetails>() ||
+ symbol->has<semantics::ObjectEntityDetails>())) {
+ continue; // recovery
+ }
+ if (IsPointer(*symbol)) { // C7104, C7105, C1594(4)
+ semantics::CheckStructConstructorPointerComponent(
+ context_, *symbol, value, innermost);
+ result.Add(*symbol, Fold(std::move(value)));
+ continue;
+ }
+ if (IsNullPointer(&value)) {
+ if (IsAllocatable(*symbol)) {
+ if (IsBareNullPointer(&value)) {
+ // NULL() with no arguments allowed by 7.5.10 para 6 for
+ // ALLOCATABLE.
+ result.Add(*symbol, Expr<SomeType>{NullPointer{}});
+ continue;
+ }
+ if (IsNullObjectPointer(&value)) {
+ AttachDeclaration(
+ Warn(common::LanguageFeature::NullMoldAllocatableComponentValue,
+ exprSource,
+ "NULL() with arguments is not standard conforming as the value for allocatable component '%s'"_port_en_US,
+ symbol->name()),
+ *symbol);
+ // proceed to check type & shape
} else {
AttachDeclaration(
- Say(expr.source,
- "A NULL pointer may not be used as the value for component '%s'"_err_en_US,
+ Say(exprSource,
+ "A NULL procedure pointer may not be used as the value for component '%s'"_err_en_US,
symbol->name()),
*symbol);
continue;
}
- } else if (IsNullAllocatable(&*value) && IsAllocatable(*symbol)) {
- result.Add(*symbol, Expr<SomeType>{NullPointer{}});
+ } else {
+ AttachDeclaration(
+ Say(exprSource,
+ "A NULL pointer may not be used as the value for component '%s'"_err_en_US,
+ symbol->name()),
+ *symbol);
continue;
- } else if (auto *derived{evaluate::GetDerivedTypeSpec(
- evaluate::DynamicType::From(*symbol))}) {
- if (auto iter{FindPointerPotentialComponent(*derived)};
- iter && pureContext) { // F'2023 C15104(4)
- if (const Symbol *
- visible{semantics::FindExternallyVisibleObject(
- *value, *pureContext)}) {
- Say(expr.source,
- "The externally visible object '%s' may not be used in a pure procedure as the value for component '%s' which has the pointer component '%s'"_err_en_US,
- visible->name(), symbol->name(),
- iter.BuildResultDesignatorName());
- } else if (ExtractCoarrayRef(*value)) {
- Say(expr.source,
- "A coindexed object may not be used in a pure procedure as the value for component '%s' which has the pointer component '%s'"_err_en_US,
- symbol->name(), iter.BuildResultDesignatorName());
- }
+ }
+ } else if (IsNullAllocatable(&value) && IsAllocatable(*symbol)) {
+ result.Add(*symbol, Expr<SomeType>{NullPointer{}});
+ continue;
+ } else if (auto *derived{evaluate::GetDerivedTypeSpec(
+ evaluate::DynamicType::From(*symbol))}) {
+ if (auto iter{FindPointerPotentialComponent(*derived)};
+ iter && pureContext) { // F'2023 C15104(4)
+ if (const Symbol *
+ visible{semantics::FindExternallyVisibleObject(
+ value, *pureContext)}) {
+ Say(exprSource,
+ "The externally visible object '%s' may not be used in a pure procedure as the value for component '%s' which has the pointer component '%s'"_err_en_US,
+ visible->name(), symbol->name(),
+ iter.BuildResultDesignatorName());
+ } else if (ExtractCoarrayRef(value)) {
+ Say(exprSource,
+ "A coindexed object may not be used in a pure procedure as the value for component '%s' which has the pointer component '%s'"_err_en_US,
+ symbol->name(), iter.BuildResultDesignatorName());
}
}
- // Make implicit conversion explicit to allow folding of the structure
- // constructors and help semantic checking, unless the component is
- // allocatable, in which case the value could be an unallocated
- // allocatable (see Fortran 2018 7.5.10 point 7). The explicit
- // convert would cause a segfault. Lowering will deal with
- // conditionally converting and preserving the lower bounds in this
- // case.
- if (MaybeExpr converted{ImplicitConvertTo(
- *symbol, std::move(*value), IsAllocatable(*symbol))}) {
- if (auto componentShape{GetShape(GetFoldingContext(), *symbol)}) {
- if (auto valueShape{GetShape(GetFoldingContext(), *converted)}) {
- if (GetRank(*componentShape) == 0 && GetRank(*valueShape) > 0) {
+ }
+ // Make implicit conversion explicit to allow folding of the structure
+ // constructors and help semantic checking, unless the component is
+ // allocatable, in which case the value could be an unallocated
+ // allocatable (see Fortran 2018 7.5.10 point 7). The explicit
+ // convert would cause a segfault. Lowering will deal with
+ // conditionally converting and preserving the lower bounds in this
+ // case.
+ if (MaybeExpr converted{ImplicitConvertTo(
+ *symbol, std::move(value), IsAllocatable(*symbol))}) {
+ if (auto componentShape{GetShape(GetFoldingContext(), *symbol)}) {
+ if (auto valueShape{GetShape(GetFoldingContext(), *converted)}) {
+ if (GetRank(*componentShape) == 0 && GetRank(*valueShape) > 0) {
+ AttachDeclaration(
+ Say(exprSource,
+ "Rank-%d array value is not compatible with scalar component '%s'"_err_en_US,
+ GetRank(*valueShape), symbol->name()),
+ *symbol);
+ } else {
+ auto checked{CheckConformance(messages, *componentShape,
+ *valueShape, CheckConformanceFlags::RightIsExpandableDeferred,
+ "component", "value")};
+ if (checked && *checked && GetRank(*componentShape) > 0 &&
+ GetRank(*valueShape) == 0 &&
+ (IsDeferredShape(*symbol) ||
+ !IsExpandableScalar(*converted, GetFoldingContext(),
+ *componentShape, true /*admit PURE call*/))) {
AttachDeclaration(
- Say(expr.source,
- "Rank-%d array value is not compatible with scalar component '%s'"_err_en_US,
- GetRank(*valueShape), symbol->name()),
+ Say(exprSource,
+ "Scalar value cannot be expanded to shape of array component '%s'"_err_en_US,
+ symbol->name()),
*symbol);
- } else {
- auto checked{
- CheckConformance(messages, *componentShape, *valueShape,
- CheckConformanceFlags::RightIsExpandableDeferred,
- "component", "value")};
- if (checked && *checked && GetRank(*componentShape) > 0 &&
- GetRank(*valueShape) == 0 &&
- (IsDeferredShape(*symbol) ||
- !IsExpandableScalar(*converted, GetFoldingContext(),
- *componentShape, true /*admit PURE call*/))) {
- AttachDeclaration(
- Say(expr.source,
- "Scalar value cannot be expanded to shape of array component '%s'"_err_en_US,
- symbol->name()),
- *symbol);
- }
- if (checked.value_or(true)) {
- result.Add(*symbol, std::move(*converted));
- }
}
- } else {
- Say(expr.source, "Shape of value cannot be determined"_err_en_US);
+ if (checked.value_or(true)) {
+ result.Add(*symbol, std::move(*converted));
+ }
}
} else {
- AttachDeclaration(
- Say(expr.source,
- "Shape of component '%s' cannot be determined"_err_en_US,
- symbol->name()),
- *symbol);
- }
- } else if (auto symType{DynamicType::From(symbol)}) {
- if (IsAllocatable(*symbol) && symType->IsUnlimitedPolymorphic() &&
- valueType) {
- // ok
- } else if (valueType) {
- AttachDeclaration(
- Say(expr.source,
- "Value in structure constructor of type '%s' is "
- "incompatible with component '%s' of type '%s'"_err_en_US,
- valueType->AsFortran(), symbol->name(),
- symType->AsFortran()),
- *symbol);
- } else {
- AttachDeclaration(
- Say(expr.source,
- "Value in structure constructor is incompatible with "
- "component '%s' of type %s"_err_en_US,
- symbol->name(), symType->AsFortran()),
- *symbol);
+ Say(exprSource, "Shape of value cannot be determined"_err_en_US);
}
+ } else {
+ AttachDeclaration(
+ Say(exprSource,
+ "Shape of component '%s' cannot be determined"_err_en_US,
+ symbol->name()),
+ *symbol);
+ }
+ } else if (auto symType{DynamicType::From(symbol)}) {
+ if (IsAllocatable(*symbol) && symType->IsUnlimitedPolymorphic() &&
+ valueType) {
+ // ok
+ } else if (valueType) {
+ AttachDeclaration(
+ Say(exprSource,
+ "Value in structure constructor of type '%s' is incompatible with component '%s' of type '%s'"_err_en_US,
+ valueType->AsFortran(), symbol->name(), symType->AsFortran()),
+ *symbol);
+ } else {
+ AttachDeclaration(
+ Say(exprSource,
+ "Value in structure constructor is incompatible with component '%s' of type %s"_err_en_US,
+ symbol->name(), symType->AsFortran()),
+ *symbol);
}
}
}
@@ -2375,10 +2353,10 @@ MaybeExpr ExpressionAnalyzer::Analyze(
} else if (IsPointer(symbol)) {
result.Add(symbol, Expr<SomeType>{NullPointer{}});
} else if (object) { // C799
- AttachDeclaration(Say(typeName,
- "Structure constructor lacks a value for "
- "component '%s'"_err_en_US,
- symbol.name()),
+ AttachDeclaration(
+ Say(typeName,
+ "Structure constructor lacks a value for component '%s'"_err_en_US,
+ symbol.name()),
symbol);
}
}
@@ -2388,6 +2366,44 @@ May...
[truncated]
|
akuhlens
approved these changes
May 9, 2025
klausler
force-pushed
the
bug138807
branch
2 times, most recently
from
b6708c4 to
e761cf3
Compare
Fortran 2023 constraint C7108 prohibits the use of a structure constructor in a way that is ambiguous with a generic function reference (intrinsic or user-defined). Sadly, no Fortran compiler implements this constraint, and the common portable interpretation seems to be the generic resolution, not the structure constructor. Restructure the processing of structure constructors in expression analysis so that it can be driven both from the parse tree as well as from generic resolution, and then use it to detect ambigous structure constructor / generic function cases, so that a portability warning can be issued. And document this as a new intentional violation of the standard in Extensions.md. Fixes llvm#138807.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fortran 2023 constraint C7108 prohibits the use of a structure constructor in a way that is ambiguous with a generic function reference (intrinsic or user-defined). Sadly, no Fortran compiler implements this constraint, and the common portable interpretation seems to be the generic resolution, not the structure constructor.
Restructure the processing of structure constructors in expression analysis so that it can be driven both from the parse tree as well as from generic resolution, and then use it to detect ambigous structure constructor / generic function cases, so that a portability warning can be issued. And document this as a new intentional violation of the standard in Extensions.md.
Fixes #138807.