From f69f7ecf48c3ab4d34c20edfd2d4bbf4b1cb5efc Mon Sep 17 00:00:00 2001 From: Tim Keith Date: Wed, 29 Aug 2018 11:38:12 -0700 Subject: [PATCH] [flang] Small improvements to name resolution Resolve names and do implicit typing in ArrayElement and LoopBounds. Fix problem with creation of UseErrorDetails: when a conflict occurs, record both the original USE and the new one. Resolve the type name in TypeAttrSpec::Extends. Move CheckUseError to DeclarationVisitor so that it can be used in ResolveDerivedType. Report error on unresolved names. I.e. after name resolution, check each parser::Name and verify we have a Symbol for it. This is on by default now but it could be an option. Original-commit: flang-compiler/f18@1c8cbc6e53cc7e1b6909bbedf5370e27fd11572f Reviewed-on: https://github.com/flang-compiler/f18/pull/173 Tree-same-pre-rewrite: false --- flang/lib/semantics/resolve-names.cc | 145 +++++++++++++++++------------- flang/lib/semantics/rewrite-parse-tree.cc | 31 ++++++- flang/lib/semantics/rewrite-parse-tree.h | 3 +- flang/lib/semantics/symbol.cc | 9 ++ flang/lib/semantics/symbol.h | 11 +-- flang/test/semantics/CMakeLists.txt | 1 + flang/test/semantics/resolve30.f90 | 4 +- flang/test/semantics/resolve31.f90 | 50 +++++++++++ 8 files changed, 182 insertions(+), 72 deletions(-) create mode 100644 flang/test/semantics/resolve31.f90 diff --git a/flang/lib/semantics/resolve-names.cc b/flang/lib/semantics/resolve-names.cc index afb1d725..239fd68 100644 --- a/flang/lib/semantics/resolve-names.cc +++ b/flang/lib/semantics/resolve-names.cc @@ -545,6 +545,7 @@ public: protected: bool BeginDecl(); void EndDecl(); + bool CheckUseError(const SourceName &, const Symbol &); private: // The attribute corresponding to the statement containing an ObjectDecl @@ -578,7 +579,7 @@ private: symbol.set_details(T(*details)); } } - if (T *details{symbol.detailsIf()}) { + if (T * details{symbol.detailsIf()}) { // OK } else if (std::is_same_v && (symbol.has() || @@ -646,6 +647,9 @@ public: void Post(const parser::Variable &x) { CheckImplicitSymbol(GetVariableName(x)); } + template void Post(const parser::LoopBounds &x) { + CheckImplicitSymbol(&x.name.thing.thing); + } bool Pre(const parser::StructureComponent &); void Post(const parser::ProcedureDesignator &); bool Pre(const parser::FunctionReference &); @@ -664,8 +668,10 @@ private: const parser::Name *GetVariableName(const parser::Expr &); const parser::Name *GetVariableName(const parser::Variable &); const Symbol *CheckImplicitSymbol(const parser::Name *); - bool CheckUseError(const SourceName &, const Symbol &); const Symbol *ResolveStructureComponent(const parser::StructureComponent &); + const Symbol *ResolveArrayElement(const parser::ArrayElement &); + const Symbol *ResolveCoindexedNamedObject( + const parser::CoindexedNamedObject &); const Symbol *ResolveDataRef(const parser::DataRef &); const Symbol *FindComponent(const Symbol &base, const SourceName &component); void CheckImports(); @@ -725,31 +731,6 @@ void ImplicitRules::SetType(const DeclTypeSpec &type, parser::Location lo, } } -const Symbol *DeclarationVisitor::ResolveDerivedType(const SourceName &name) { - auto *symbol{FindSymbol(name)}; - if (!symbol) { - Say(name, "Derived type '%s' not found"_err_en_US); - return nullptr; - } - if (auto *details{symbol->detailsIf()}) { - const Symbol &useSymbol{details->symbol()}; - if (const auto *details{useSymbol.detailsIf()}) { - if (details->derivedType()) { - return details->derivedType(); - } - } - return &useSymbol; - } - if (auto *details{symbol->detailsIf()}) { - if (details->derivedType()) { - symbol->remove_occurrence(name); - symbol = details->derivedType(); - details->derivedType()->add_occurrence(name); - } - } - return symbol; -} - // Return the next char after ch in a way that works for ASCII or EBCDIC. // Return '\0' for the char after 'z'. char ImplicitRules::Incr(char ch) { @@ -1324,18 +1305,16 @@ void ModuleVisitor::AddUse(const SourceName &location, localSymbol.attrs() &= ~Attrs{Attr::PUBLIC, Attr::PRIVATE}; localSymbol.flags() |= useSymbol.flags(); if (auto *details{localSymbol.detailsIf()}) { - // check for importing the same symbol again: + // check for use-associating the same symbol again: if (localSymbol.GetUltimate() != useSymbol.GetUltimate()) { localSymbol.set_details( - UseErrorDetails{details->location(), *useModuleScope_}); + UseErrorDetails{*details}.add_occurrence(location, *useModuleScope_)); } } else if (auto *details{localSymbol.detailsIf()}) { details->add_occurrence(location, *useModuleScope_); - } else if (localSymbol.has()) { - localSymbol.set_details(UseDetails{location, useSymbol}); } else { - localSymbol.set_details( - UseErrorDetails{useSymbol.name(), *useModuleScope_}); + CHECK(localSymbol.has()); + localSymbol.set_details(UseDetails{location, useSymbol}); } } @@ -1879,6 +1858,22 @@ void DeclarationVisitor::EndDecl() { EndArraySpec(); } +bool DeclarationVisitor::CheckUseError( + const SourceName &name, const Symbol &symbol) { + const auto *details{symbol.detailsIf()}; + if (!details) { + return false; + } + Message &msg{Say(name, "Reference to '%s' is ambiguous"_err_en_US)}; + for (const auto &pair : details->occurrences()) { + const SourceName &location{*pair.first}; + const SourceName &moduleName{pair.second->name()}; + msg.Attach(location, "'%s' was use-associated from module '%s'"_en_US, + name.ToString().data(), moduleName.ToString().data()); + } + return true; +} + void DeclarationVisitor::Post(const parser::DimensionStmt::Declaration &x) { const auto &name{std::get(x.t)}; DeclareObjectEntity(name, Attrs{}); @@ -2023,10 +2018,6 @@ void DeclarationVisitor::Post(const parser::DeclarationTypeSpec::Type &x) { SetDerivedDeclTypeSpec(DeclTypeSpec::TypeDerived); DerivedTypeSpec &type{GetDeclTypeSpec()->derivedTypeSpec()}; if (const auto *symbol{ResolveDerivedType(type.name())}) { - if (!symbol->has()) { - Say(type.name(), "'%s' is not a derived type"_err_en_US); - return; - } type.set_scope(*symbol->scope()); } } @@ -2059,7 +2050,11 @@ bool DeclarationVisitor::Pre(const parser::EndTypeStmt &) { return false; } bool DeclarationVisitor::Pre(const parser::TypeAttrSpec::Extends &x) { - derivedTypeData_->extends = &x.v.source; + auto &name{x.v.source}; + ResolveDerivedType(name); + // TODO: use resolved symbol in derived type + // const auto *symbol{ResolveDerivedType(name)}; + derivedTypeData_->extends = &name; return false; } bool DeclarationVisitor::Pre(const parser::PrivateStmt &x) { @@ -2137,6 +2132,30 @@ void DeclarationVisitor::SetType( symbol.SetType(type); } +const Symbol *DeclarationVisitor::ResolveDerivedType(const SourceName &name) { + const auto *symbol{FindSymbol(name)}; + if (!symbol) { + Say(name, "Derived type '%s' not found"_err_en_US); + return nullptr; + } + if (CheckUseError(name, *symbol)) { + return nullptr; + } + if (auto *details{symbol->detailsIf()}) { + symbol = &details->symbol(); + } + if (auto *details{symbol->detailsIf()}) { + if (details->derivedType()) { + symbol = details->derivedType(); + } + } + if (!symbol->has()) { + Say(name, "'%s' is not a derived type"_err_en_US); + return nullptr; + } + return symbol; +} + // ResolveNamesVisitor implementation bool ResolveNamesVisitor::Pre(const parser::TypeParamDefStmt &x) { @@ -2212,22 +2231,6 @@ bool ResolveNamesVisitor::Pre(const parser::ImportStmt &x) { return false; } -bool ResolveNamesVisitor::CheckUseError( - const SourceName &name, const Symbol &symbol) { - const auto *details{symbol.detailsIf()}; - if (!details) { - return false; - } - Message &msg{Say(name, "Reference to '%s' is ambiguous"_err_en_US)}; - for (const auto &pair : details->occurrences()) { - const SourceName &location{*pair.first}; - const SourceName &moduleName{pair.second->name()}; - msg.Attach(location, "'%s' was use-associated from module '%s'"_en_US, - name.ToString().data(), moduleName.ToString().data()); - } - return true; -} - bool ResolveNamesVisitor::Pre(const parser::StructureComponent &x) { ResolveStructureComponent(x); return false; @@ -2238,6 +2241,14 @@ const Symbol *ResolveNamesVisitor::ResolveStructureComponent( const Symbol *dataRef = ResolveDataRef(x.base); return dataRef ? FindComponent(*dataRef, x.component.source) : nullptr; } +const Symbol *ResolveNamesVisitor::ResolveArrayElement( + const parser::ArrayElement &x) { + return ResolveDataRef(x.base); +} +const Symbol *ResolveNamesVisitor::ResolveCoindexedNamedObject( + const parser::CoindexedNamedObject &x) { + return nullptr; // TODO +} const Symbol *ResolveNamesVisitor::ResolveDataRef(const parser::DataRef &x) { return std::visit( @@ -2259,8 +2270,11 @@ const Symbol *ResolveNamesVisitor::ResolveDataRef(const parser::DataRef &x) { [=](const common::Indirection &y) { return ResolveStructureComponent(*y); }, - [](const auto &) { - return static_cast(nullptr); // TODO + [=](const common::Indirection &y) { + return ResolveArrayElement(*y); + }, + [=](const common::Indirection &y) { + return ResolveCoindexedNamedObject(*y); }, }, x.u); @@ -2532,15 +2546,26 @@ bool ResolveNamesVisitor::Pre(const parser::ImplicitStmt &x) { const parser::Name *ResolveNamesVisitor::GetVariableName( const parser::DataRef &x) { - return std::get_if(&x.u); + return std::visit( + common::visitors{ + [](const parser::Name &x) { return &x; }, + [&](const common::Indirection &x) { + return GetVariableName(x->base); + }, + [](const auto &) { + return static_cast(nullptr); + }, + }, + x.u); } + const parser::Name *ResolveNamesVisitor::GetVariableName( const parser::Designator &x) { return std::visit( common::visitors{ - [&](const parser::ObjectName &x) { return &x; }, + [](const parser::ObjectName &x) { return &x; }, [&](const parser::DataRef &x) { return GetVariableName(x); }, - [&](const auto &) { + [](const auto &) { return static_cast(nullptr); }, }, @@ -2614,7 +2639,7 @@ void ResolveNames(Scope &rootScope, parser::Program &program, visitor.messages().Emit(std::cerr, cookedSource); return; } - RewriteParseTree(program); + RewriteParseTree(program, cookedSource); } // Map the enum in the parser to the one in GenericSpec diff --git a/flang/lib/semantics/rewrite-parse-tree.cc b/flang/lib/semantics/rewrite-parse-tree.cc index 5e9ee20..e793ae0 100644 --- a/flang/lib/semantics/rewrite-parse-tree.cc +++ b/flang/lib/semantics/rewrite-parse-tree.cc @@ -22,6 +22,8 @@ namespace Fortran::semantics { +using namespace parser::literals; + // Symbols collected during name resolution that are added to parse tree. using symbolMap = std::map; @@ -31,6 +33,8 @@ class RewriteMutator { public: RewriteMutator(const symbolMap &symbols) : symbols_{symbols} {} + const parser::Messages &messages() const { return messages_; } + // Default action for a parse tree node is to visit children. template bool Pre(T &) { return true; } template void Post(T &) {} @@ -40,6 +44,9 @@ public: const auto it{symbols_.find(name.source.begin())}; if (it != symbols_.end()) { name.symbol = it->second; + } else if (errorOnUnresolvedName_) { + messages_.Say(name.source, "Internal: no symbol found for '%s'"_err_en_US, + name.ToString().c_str()); } } @@ -80,9 +87,29 @@ public: void Post(parser::Expr &x) { ConvertFunctionRef(x); } + // Don't bother resolving names in end statements. + bool Pre(parser::EndAssociateStmt &) { return false; } + bool Pre(parser::EndBlockDataStmt &) { return false; } + bool Pre(parser::EndBlockStmt &) { return false; } + bool Pre(parser::EndCriticalStmt &) { return false; } + bool Pre(parser::EndDoStmt &) { return false; } + bool Pre(parser::EndForallStmt &) { return false; } + bool Pre(parser::EndFunctionStmt &) { return false; } + bool Pre(parser::EndIfStmt &) { return false; } + bool Pre(parser::EndModuleStmt &) { return false; } + bool Pre(parser::EndMpSubprogramStmt &) { return false; } + bool Pre(parser::EndProgramStmt &) { return false; } + bool Pre(parser::EndSelectStmt &) { return false; } + bool Pre(parser::EndSubmoduleStmt &) { return false; } + bool Pre(parser::EndSubroutineStmt &) { return false; } + bool Pre(parser::EndTypeStmt &) { return false; } + bool Pre(parser::EndWhereStmt &) { return false; } + private: + bool errorOnUnresolvedName_{true}; const symbolMap &symbols_; std::list stmtFuncsToConvert; + parser::Messages messages_; // For T = Variable or Expr, if x has a function reference that really // should be an array element reference (i.e. the name occurs in an @@ -123,11 +150,13 @@ static void CollectSymbols(Scope &scope, symbolMap &symbols) { } } -void RewriteParseTree(parser::Program &program) { +void RewriteParseTree( + parser::Program &program, const parser::CookedSource &cookedSource) { symbolMap symbols; CollectSymbols(Scope::globalScope, symbols); RewriteMutator mutator{symbols}; parser::Walk(program, mutator); + mutator.messages().Emit(std::cerr, cookedSource); } } // namespace Fortran::semantics diff --git a/flang/lib/semantics/rewrite-parse-tree.h b/flang/lib/semantics/rewrite-parse-tree.h index f2f93b5..c1d0ef1 100644 --- a/flang/lib/semantics/rewrite-parse-tree.h +++ b/flang/lib/semantics/rewrite-parse-tree.h @@ -17,10 +17,11 @@ namespace Fortran::parser { struct Program; +struct CookedSource; } // namespace Fortran::parser namespace Fortran::semantics { -void RewriteParseTree(parser::Program &); +void RewriteParseTree(parser::Program &, const parser::CookedSource &); } // namespace Fortran::semantics #endif // FORTRAN_SEMANTICS_REWRITE_PARSE_TREE_H_ diff --git a/flang/lib/semantics/symbol.cc b/flang/lib/semantics/symbol.cc index 3e2a995..4e5482d 100644 --- a/flang/lib/semantics/symbol.cc +++ b/flang/lib/semantics/symbol.cc @@ -73,6 +73,15 @@ const Symbol &UseDetails::module() const { return *symbol_->owner().symbol(); } +UseErrorDetails::UseErrorDetails(const UseDetails &useDetails) { + add_occurrence(useDetails.location(), *useDetails.module().scope()); +} +UseErrorDetails &UseErrorDetails::add_occurrence( + const SourceName &location, const Scope &module) { + occurrences_.push_back(std::make_pair(&location, &module)); + return *this; +} + GenericDetails::GenericDetails(const listType &specificProcs) { for (const auto *proc : specificProcs) { add_specificProc(proc); diff --git a/flang/lib/semantics/symbol.h b/flang/lib/semantics/symbol.h index cfefe24..75ed220 100644 --- a/flang/lib/semantics/symbol.h +++ b/flang/lib/semantics/symbol.h @@ -165,15 +165,8 @@ private: // we can report the error if it is used. class UseErrorDetails { public: - UseErrorDetails(const SourceName &location, const Scope &module) { - add_occurrence(location, module); - } - - UseErrorDetails &add_occurrence( - const SourceName &location, const Scope &module) { - occurrences_.push_back(std::make_pair(&location, &module)); - return *this; - } + UseErrorDetails(const UseDetails &); + UseErrorDetails &add_occurrence(const SourceName &, const Scope &); using listType = std::list>; const listType occurrences() const { return occurrences_; }; diff --git a/flang/test/semantics/CMakeLists.txt b/flang/test/semantics/CMakeLists.txt index d12a052..c1fc5a2 100644 --- a/flang/test/semantics/CMakeLists.txt +++ b/flang/test/semantics/CMakeLists.txt @@ -55,6 +55,7 @@ set(ERROR_TESTS resolve28.f90 resolve29.f90 resolve30.f90 + resolve31.f90 ) # These test files have expected symbols in the source diff --git a/flang/test/semantics/resolve30.f90 b/flang/test/semantics/resolve30.f90 index 1f7c860..00fe442 100644 --- a/flang/test/semantics/resolve30.f90 +++ b/flang/test/semantics/resolve30.f90 @@ -35,7 +35,9 @@ subroutine s3 import, only: j type t !ERROR: 'i' from host scoping unit is not accessible due to IMPORT - real :: x(10) = [(i, i=1,10)] + real :: x(10) = [(i, & + !ERROR: 'i' from host scoping unit is not accessible due to IMPORT + i=1,10)] end type end block end subroutine diff --git a/flang/test/semantics/resolve31.f90 b/flang/test/semantics/resolve31.f90 new file mode 100644 index 0000000..f723764 --- /dev/null +++ b/flang/test/semantics/resolve31.f90 @@ -0,0 +1,50 @@ +! Copyright (c) 2018, NVIDIA CORPORATION. All rights reserved. +! +! Licensed under the Apache License, Version 2.0 (the "License"); +! you may not use this file except in compliance with the License. +! You may obtain a copy of the License at +! +! http://www.apache.org/licenses/LICENSE-2.0 +! +! Unless required by applicable law or agreed to in writing, software +! distributed under the License is distributed on an "AS IS" BASIS, +! WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +! See the License for the specific language governing permissions and +! limitations under the License. + +subroutine s1 + integer :: t0 + !ERROR: 't0' is not a derived type + type(t0) :: x + type :: t1 + end type + type, extends(t1) :: t2 + end type + !ERROR: Derived type 't3' not found + type, extends(t3) :: t4 + end type + !ERROR: 't0' is not a derived type + type, extends(t0) :: t5 + end type +end subroutine + +module m1 + type t0 + end type +end +module m2 + type t + end type +end +module m3 + type t0 + end type +end +subroutine s2 + use m1 + use m2, t0 => t + use m3 + !ERROR: Reference to 't0' is ambiguous + type, extends(t0) :: t1 + end type +end subroutine -- 2.7.4