From facd7147f412dc7e6a94079bb2b323d8851236b7 Mon Sep 17 00:00:00 2001
From: Tim Keith <tkeith@nvidia.com>
Date: Mon, 24 Jun 2019 15:22:57 -0700
Subject: [PATCH] [flang] Fix internal error due to missing type

If a name is declared in a module with just `private :: x`,
we don't know if `x` is supposed to be an object or procedure,
so it ends up not getting an implicit type. This leads to an
internal error when writing the `.mod` file.

The fix requires two changes:
1. Cause all `EntityDetails` to be converted to `ObjectEntityDetails`
   in `ResolveSpecificationParts`. This is done by calling `PopScope()`.
   The reason for not calling it there no longer applies since the
   addition of the `ExecutionPartSkimmer` pass.
2. In the same place, apply the implicit typing rules to every entity
   in the scope.

Fixes flang-compiler/f18#514, fixes flang-compiler/f18#520.

Original-commit: flang-compiler/f18@7e4185e9ff34dc02f6299aa16236f3b932b16652
Reviewed-on: https://github.com/flang-compiler/f18/pull/524
---
 flang/lib/semantics/resolve-names.cc | 40 +++++++++-------------------
 flang/test/semantics/CMakeLists.txt  |  3 ++-
 flang/test/semantics/modfile29.f90   | 30 +++++++++++++++++++++
 3 files changed, 45 insertions(+), 28 deletions(-)
 create mode 100644 flang/test/semantics/modfile29.f90

diff --git a/flang/lib/semantics/resolve-names.cc b/flang/lib/semantics/resolve-names.cc
index 83d2e0db7f48..1c3ec0427437 100644
--- a/flang/lib/semantics/resolve-names.cc
+++ b/flang/lib/semantics/resolve-names.cc
@@ -1669,18 +1669,15 @@ void ScopeHandler::EraseSymbol(const parser::Name &name) {
 }
 
 static bool NeedsType(const Symbol &symbol) {
-  if (symbol.GetType()) {
-    return false;
-  }
-  if (auto *details{symbol.detailsIf<ProcEntityDetails>()}) {
-    if (details->interface().symbol()) {
-      return false;  // the interface determines the type
-    }
-    if (!symbol.test(Symbol::Flag::Function)) {
-      return false;  // not known to be a function
-    }
-  }
-  return true;
+  return symbol.GetType() == nullptr &&
+      std::visit(
+          common::visitors{
+              [](const EntityDetails &) { return true; },
+              [](const ObjectEntityDetails &) { return true; },
+              [](const AssocEntityDetails &) { return true; },
+              [](const auto &) { return false; },
+          },
+          symbol.details());
 }
 void ScopeHandler::ApplyImplicitRules(Symbol &symbol) {
   if (NeedsType(symbol)) {
@@ -4835,22 +4832,11 @@ void ResolveNamesVisitor::ResolveSpecificationParts(ProgramTree &node) {
     ResolveSpecificationParts(child);
   }
   ExecutionPartSkimmer{scope}.Walk(node.exec());
-  // Convert function results and dummy arguments to objects if we don't
-  // already known by now that they're procedures.
-  if (currScope().kind() == Scope::Kind::Subprogram) {
-    for (const auto &pair : currScope()) {
-      Symbol &symbol{*pair.second};
-      if (auto *details{symbol.detailsIf<EntityDetails>()}) {
-        if (details->isFuncResult() || details->isDummy()) {
-          ConvertToObjectEntity(symbol);
-        }
-      }
-    }
+  PopScope();
+  // Ensure every object entity has a type:
+  for (auto &pair : *node.scope()) {
+    ApplyImplicitRules(*pair.second);
   }
-  // Subtlety: PopScope() is not called here because we want to defer
-  // conversions of other uncategorized entities into objects until after
-  // we have traversed the executable part of the subprogram.
-  SetScope(currScope().parent());
 }
 
 // Add SubprogramNameDetails symbols for contained subprograms
diff --git a/flang/test/semantics/CMakeLists.txt b/flang/test/semantics/CMakeLists.txt
index b19dd471f611..78fbab448935 100644
--- a/flang/test/semantics/CMakeLists.txt
+++ b/flang/test/semantics/CMakeLists.txt
@@ -129,7 +129,7 @@ set(ERROR_TESTS
   expr-errors01.f90
   null01.f90
   equivalence01.f90
-  # ${PROJECT_SOURCE_DIR}/module/ieee_arithmetic.f90 #520
+  ${PROJECT_SOURCE_DIR}/module/ieee_arithmetic.f90
   ${PROJECT_SOURCE_DIR}/module/ieee_exceptions.f90
   ${PROJECT_SOURCE_DIR}/module/ieee_features.f90
   ${PROJECT_SOURCE_DIR}/module/iso_c_binding.f90
@@ -185,6 +185,7 @@ set(MODFILE_TESTS
   modfile26.f90
   modfile27.f90
   modfile28.f90
+  modfile29.f90
 )
 
 set(LABEL_TESTS
diff --git a/flang/test/semantics/modfile29.f90 b/flang/test/semantics/modfile29.f90
new file mode 100644
index 000000000000..e776d6122db6
--- /dev/null
+++ b/flang/test/semantics/modfile29.f90
@@ -0,0 +1,30 @@
+! Copyright (c) 2019, 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.
+
+! Check that implicitly typed entities get a type in the module file.
+
+module m
+  public :: a
+  private :: b
+  protected :: i
+  allocatable :: j
+end
+
+!Expect: m.mod
+!module m
+! real(4)::a
+! real(4),private::b
+! integer(4),protected::i
+! integer(4),allocatable::j
+!end