[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@1c8cbc6e53
Reviewed-on: https://github.com/flang-compiler/f18/pull/173
Tree-same-pre-rewrite: false

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<T>()}) {
+    if (T * details{symbol.detailsIf<T>()}) {
       // OK
     } else if (std::is_same_v<EntityDetails, T> &&
         (symbol.has<ObjectEntityDetails>() ||
@@ -646,6 +647,9 @@ public:
   void Post(const parser::Variable &x) {
     CheckImplicitSymbol(GetVariableName(x));
   }
+  template<typename T> void Post(const parser::LoopBounds<T> &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<UseDetails>()}) {
-    const Symbol &useSymbol{details->symbol()};
-    if (const auto *details{useSymbol.detailsIf<GenericDetails>()}) {
-      if (details->derivedType()) {
-        return details->derivedType();
-      }
-    }
-    return &useSymbol;
-  }
-  if (auto *details{symbol->detailsIf<GenericDetails>()}) {
-    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<UseDetails>()}) {
-    // 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<UseErrorDetails>()}) {
     details->add_occurrence(location, *useModuleScope_);
-  } else if (localSymbol.has<UnknownDetails>()) {
-    localSymbol.set_details(UseDetails{location, useSymbol});
   } else {
-    localSymbol.set_details(
-        UseErrorDetails{useSymbol.name(), *useModuleScope_});
+    CHECK(localSymbol.has<UnknownDetails>());
+    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<UseErrorDetails>()};
+  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<parser::Name>(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<DerivedTypeDetails>()) {
-      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<UseDetails>()}) {
+    symbol = &details->symbol();
+  }
+  if (auto *details{symbol->detailsIf<GenericDetails>()}) {
+    if (details->derivedType()) {
+      symbol = details->derivedType();
+    }
+  }
+  if (!symbol->has<DerivedTypeDetails>()) {
+    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<UseErrorDetails>()};
-  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<parser::StructureComponent> &y) {
             return ResolveStructureComponent(*y);
           },
-          [](const auto &) {
-            return static_cast<const Symbol *>(nullptr);  // TODO
+          [=](const common::Indirection<parser::ArrayElement> &y) {
+            return ResolveArrayElement(*y);
+          },
+          [=](const common::Indirection<parser::CoindexedNamedObject> &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<parser::Name>(&x.u);
+  return std::visit(
+      common::visitors{
+          [](const parser::Name &x) { return &x; },
+          [&](const common::Indirection<parser::ArrayElement> &x) {
+            return GetVariableName(x->base);
+          },
+          [](const auto &) {
+            return static_cast<const parser::Name *>(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<const parser::Name *>(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

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<const char *, Symbol *>;
 
@@ -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<typename T> bool Pre(T &) { return true; }
   template<typename T> 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<stmtFuncType> 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

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_

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);

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<std::pair<const SourceName *, const Scope *>>;
   const listType occurrences() const { return occurrences_; };

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

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

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