[flang] Refactoring

Eliminate two of the three overloadings of `Scope::MakeDerivedType()`.
Keep the one that has `Category` first because that's the order the
constructor of `DeclTypeSpec` uses.

In `mod-file.cc`, eliminate some calls to `PutLower()`. Symbols and
names are already lower case so they don't need to be converted.

In `modfile03.f90`, move the expected `.mod` file comments to right
after the corresponding modules. That makes it easier to work with.

Original-commit: flang-compiler/f18@86874d9bf8
Reviewed-on: https://github.com/flang-compiler/f18/pull/740
Tree-same-pre-rewrite: false

flang/lib/semantics/mod-file.cc

@@ -53,8 +53,7 @@ static std::ostream &PutAttrs(std::ostream &, Attrs,
     const MaybeExpr & = std::nullopt, std::string before = ","s,
     std::string after = ""s);
 static std::ostream &PutAttr(std::ostream &, Attr);
-static std::ostream &PutLower(std::ostream &, const Symbol &);
-static std::ostream &PutLower(std::ostream &, const DeclTypeSpec &);
+static std::ostream &PutType(std::ostream &, const DeclTypeSpec &);
 static std::ostream &PutLower(std::ostream &, const std::string &);
 static bool WriteFile(const std::string &, std::string &&);
 static bool FileContentsMatch(
@@ -142,15 +141,15 @@ std::string ModFileWriter::GetAsString(const Symbol &symbol) {
   std::stringstream all;
   auto &details{symbol.get<ModuleDetails>()};
   if (!details.isSubmodule()) {
-    PutLower(all << "module ", symbol);
+    all << "module " << symbol.name();
   } else {
     auto *parent{details.parent()->symbol()};
     auto *ancestor{details.ancestor()->symbol()};
-    PutLower(all << "submodule(", *ancestor);
+    all << "submodule(" << ancestor->name();
     if (parent != ancestor) {
-      PutLower(all << ':', *parent);
+      all << ':' << parent->name();
     }
-    PutLower(all << ") ", symbol);
+    all << ") " << symbol.name();
   }
   all << '\n' << uses_.str();
   uses_.str(""s);
@@ -202,46 +201,46 @@ void ModFileWriter::PutSymbol(
             bool deferred{symbol->attrs().test(Attr::DEFERRED)};
             typeBindings << "procedure";
             if (deferred) {
-              PutLower(typeBindings << '(', x.symbol()) << ')';
+              typeBindings << '(' << x.symbol().name() << ')';
             }
             PutPassName(typeBindings, x.passName());
             PutAttrs(typeBindings, symbol->attrs());
-            PutLower(typeBindings << "::", *symbol);
+            typeBindings << "::" << symbol->name();
             if (!deferred && x.symbol().name() != symbol->name()) {
-              PutLower(typeBindings << "=>", x.symbol());
+              typeBindings << "=>" << x.symbol().name();
             }
             typeBindings << '\n';
           },
           [&](const GenericBindingDetails &x) {
             for (const auto *proc : x.specificProcs()) {
-              PutLower(typeBindings << "generic::", *symbol);
-              PutLower(typeBindings << "=>", *proc) << '\n';
+              typeBindings << "generic::" << symbol->name() << "=>"
+                           << proc->name() << '\n';
             }
           },
           [&](const NamelistDetails &x) {
-            PutLower(decls_ << "namelist/", *symbol);
+            decls_ << "namelist/" << symbol->name();
             char sep{'/'};
             for (const auto *object : x.objects()) {
-              PutLower(decls_ << sep, *object);
+              decls_ << sep << object->name();
               sep = ',';
             }
             decls_ << '\n';
           },
           [&](const CommonBlockDetails &x) {
-            PutLower(decls_ << "common/", *symbol);
+            decls_ << "common/" << symbol->name();
             char sep = '/';
             for (const auto *object : x.objects()) {
-              PutLower(decls_ << sep, *object);
+              decls_ << sep << object->name();
               sep = ',';
             }
             decls_ << '\n';
             if (symbol->attrs().test(Attr::BIND_C)) {
               PutAttrs(decls_, symbol->attrs(), x.bindName(), ""s);
-              PutLower(decls_ << "::/", *symbol) << "/\n";
+              decls_ << "::/" << symbol->name() << "/\n";
             }
           },
           [&](const FinalProcDetails &) {
-            PutLower(typeBindings << "final::", *symbol) << '\n';
+            typeBindings << "final::" << symbol->name() << '\n';
           },
           [](const HostAssocDetails &) {},
           [](const MiscDetails &) {},
@@ -254,16 +253,15 @@ void ModFileWriter::PutDerivedType(const Symbol &typeSymbol) {
   auto &details{typeSymbol.get<DerivedTypeDetails>()};
   PutAttrs(decls_ << "type", typeSymbol.attrs());
   if (const DerivedTypeSpec * extends{typeSymbol.GetParentTypeSpec()}) {
-    PutLower(decls_ << ",extends(", extends->typeSymbol()) << ')';
+    decls_ << ",extends(" << extends->typeSymbol().name() << ')';
   }
-  PutLower(decls_ << "::", typeSymbol);
+  decls_ << "::" << typeSymbol.name();
   auto &typeScope{*typeSymbol.scope()};
   if (!details.paramNames().empty()) {
-    bool first{true};
-    decls_ << '(';
+    char sep{'('};
     for (const auto &name : details.paramNames()) {
-      PutLower(first ? decls_ : decls_ << ',', name.ToString());
-      first = false;
+      decls_ << sep << name;
+      sep = ',';
     }
     decls_ << ')';
   }
@@ -294,8 +292,7 @@ void ModFileWriter::PutSubprogram(const Symbol &symbol) {
   std::stringstream ss;
   PutAttrs(ss, attrs);
   if (!ss.str().empty()) {
-    decls_ << ss.str().substr(1) << "::";
-    PutLower(decls_, symbol) << "\n";
+    decls_ << ss.str().substr(1) << "::" << symbol.name() << '\n';
   }
   bool isInterface{details.isInterface()};
   std::ostream &os{isInterface ? decls_ : contains_};
@@ -304,20 +301,20 @@ void ModFileWriter::PutSubprogram(const Symbol &symbol) {
   }
   PutAttrs(os, prefixAttrs, std::nullopt, ""s, " "s);
   os << (details.isFunction() ? "function " : "subroutine ");
-  PutLower(os, symbol) << '(';
+  os << symbol.name() << '(';
   int n = 0;
   for (const auto &dummy : details.dummyArgs()) {
     if (n++ > 0) {
       os << ',';
     }
-    PutLower(os, *dummy);
+    os << dummy->name();
   }
   os << ')';
   PutAttrs(os, bindAttrs, details.bindName(), " "s, ""s);
   if (details.isFunction()) {
     const Symbol &result{details.result()};
     if (result.name() != symbol.name()) {
-      PutLower(os << " result(", result) << ')';
+      os << " result(" << result.name() << ')';
     }
   }
   os << '\n';
@@ -345,9 +342,9 @@ void ModFileWriter::PutSubprogram(const Symbol &symbol) {
 static std::ostream &PutGenericName(std::ostream &os, const Symbol &symbol) {
   const auto *details{symbol.GetUltimate().detailsIf<GenericDetails>()};
   if (details && details->kind() == GenericKind::DefinedOp) {
-    return PutLower(os << "operator(", symbol) << ')';
+    return os << "operator(" << symbol.name() << ')';
   } else {
-    return PutLower(os, symbol);
+    return os << symbol.name();
   }
 }
 
@@ -355,7 +352,7 @@ void ModFileWriter::PutGeneric(const Symbol &symbol) {
   auto &details{symbol.get<GenericDetails>()};
   PutGenericName(decls_ << "interface ", symbol) << '\n';
   for (auto *specific : details.specificProcs()) {
-    PutLower(decls_ << "procedure::", DEREF(specific)) << '\n';
+    decls_ << "procedure::" << specific->name() << '\n';
   }
   decls_ << "end interface\n";
   if (symbol.attrs().test(Attr::PRIVATE)) {
@@ -366,7 +363,7 @@ void ModFileWriter::PutGeneric(const Symbol &symbol) {
 void ModFileWriter::PutUse(const Symbol &symbol) {
   auto &details{symbol.get<UseDetails>()};
   auto &use{details.symbol()};
-  PutLower(uses_ << "use ", details.module());
+  uses_ << "use " << details.module().name();
   PutGenericName(uses_ << ",only:", symbol);
   if (use.name() != symbol.name()) {
     PutGenericName(uses_ << "=>", use);
@@ -382,7 +379,7 @@ void ModFileWriter::PutUseExtraAttr(
     Attr attr, const Symbol &local, const Symbol &use) {
   if (local.attrs().test(attr) && !use.attrs().test(attr)) {
     PutAttr(useExtraAttrs_, attr) << "::";
-    PutLower(useExtraAttrs_, local) << '\n';
+    useExtraAttrs_ << local.name() << '\n';
   }
 }
 
@@ -472,7 +469,7 @@ void PutShape(std::ostream &os, const ArraySpec &shape, char open, char close) {
 
 void PutObjectEntity(std::ostream &os, const Symbol &symbol) {
   auto &details{symbol.get<ObjectEntityDetails>()};
-  PutEntity(os, symbol, [&]() { PutLower(os, DEREF(symbol.GetType())); });
+  PutEntity(os, symbol, [&]() { PutType(os, DEREF(symbol.GetType())); });
   PutShape(os, details.shape(), '(', ')');
   PutShape(os, details.coshape(), '[', ']');
   PutInit(os, symbol, details.init());
@@ -488,9 +485,9 @@ void PutProcEntity(std::ostream &os, const Symbol &symbol) {
   PutEntity(os, symbol, [&]() {
     os << "procedure(";
     if (interface.symbol()) {
-      PutLower(os, *interface.symbol());
+      os << interface.symbol()->name();
     } else if (interface.type()) {
-      PutLower(os, *interface.type());
+      PutType(os, *interface.type());
     }
     os << ')';
     PutPassName(os, details.passName());
@@ -500,14 +497,14 @@ void PutProcEntity(std::ostream &os, const Symbol &symbol) {
 
 void PutPassName(std::ostream &os, const std::optional<SourceName> &passName) {
   if (passName) {
-    PutLower(os << ",pass(", passName->ToString()) << ')';
+    os << ",pass(" << *passName << ')';
   }
 }
 
 void PutTypeParam(std::ostream &os, const Symbol &symbol) {
   auto &details{symbol.get<TypeParamDetails>()};
   PutEntity(os, symbol, [&]() {
-    PutLower(os, DEREF(symbol.GetType()));
+    PutType(os, DEREF(symbol.GetType()));
     PutLower(os << ',', common::EnumToString(details.attr()));
   });
   PutInit(os, details.init());
@@ -555,7 +552,7 @@ void PutEntity(
       },
       symbol.details());
   PutAttrs(os, symbol.attrs(), bindName);
-  PutLower(os << "::", symbol);
+  os << "::" << symbol.name();
 }
 
 // Put out each attribute to os, surrounded by `before` and `after` and
@@ -581,11 +578,7 @@ std::ostream &PutAttr(std::ostream &os, Attr attr) {
   return PutLower(os, AttrToString(attr));
 }
 
-std::ostream &PutLower(std::ostream &os, const Symbol &symbol) {
-  return PutLower(os, symbol.name().ToString());
-}
-
-std::ostream &PutLower(std::ostream &os, const DeclTypeSpec &type) {
+std::ostream &PutType(std::ostream &os, const DeclTypeSpec &type) {
   return PutLower(os, type.AsFortran());
 }
 
@@ -793,9 +786,9 @@ static std::string ModFilePath(const std::string &dir, const SourceName &name,
     path << dir << '/';
   }
   if (!ancestorName.empty()) {
-    PutLower(path, ancestorName) << '-';
+    path << ancestorName << '-';
   }
-  PutLower(path, name.ToString()) << suffix;
+  path << name << suffix;
   return path.str();
 }
 

flang/lib/semantics/resolve-names.cc

@@ -3243,7 +3243,7 @@ void DeclarationVisitor::Post(const parser::DerivedTypeSpec &x) {
     // in this scope.
     SetDeclTypeSpec(*extant);
   } else {
-    DeclTypeSpec &type{currScope().MakeDerivedType(std::move(spec), category)};
+    DeclTypeSpec &type{currScope().MakeDerivedType(category, std::move(spec))};
     if (parameterNames.empty() || currScope().IsParameterizedDerivedType()) {
       // The derived type being instantiated is not a parameterized derived
       // type, or the instantiation is within the definition of a parameterized
@@ -3350,7 +3350,8 @@ void DeclarationVisitor::Post(const parser::DerivedTypeStmt &x) {
       auto &comp{DeclareEntity<ObjectEntityDetails>(*extendsName, Attrs{})};
       comp.attrs().set(Attr::PRIVATE, extendsType->attrs().test(Attr::PRIVATE));
       comp.set(Symbol::Flag::ParentComp);
-      DeclTypeSpec &type{currScope().MakeDerivedType(*extendsType)};
+      DeclTypeSpec &type{currScope().MakeDerivedType(
+          DeclTypeSpec::TypeDerived, DerivedTypeSpec{*extendsType})};
       type.derivedTypeSpec().set_scope(*extendsType->scope());
       comp.SetType(type);
       DerivedTypeDetails &details{symbol.get<DerivedTypeDetails>()};
@@ -4792,10 +4793,12 @@ const DeclTypeSpec &ConstructVisitor::ToDeclTypeSpec(
     } else if (type.IsUnlimitedPolymorphic()) {
       return currScope().MakeClassStarType();
     } else {
-      return currScope().MakeDerivedType(type.IsPolymorphic()
-              ? DeclTypeSpec::ClassDerived
-              : DeclTypeSpec::TypeDerived,
-          DerivedTypeSpec{type.GetDerivedTypeSpec()});
+      return currScope().MakeDerivedType(
+          type.IsPolymorphic() ? DeclTypeSpec::ClassDerived
+                               : DeclTypeSpec::TypeDerived,
+          common::Clone(type.GetDerivedTypeSpec())
+
+      );
     }
   case common::TypeCategory::Character:
   default: CRASH_NO_CASE;

flang/lib/semantics/scope.cc

@@ -155,22 +155,9 @@ const DeclTypeSpec &Scope::MakeCharacterType(
       CharacterTypeSpec{std::move(length), std::move(kind)});
 }
 
-const DeclTypeSpec &Scope::MakeDerivedType(
-    DeclTypeSpec::Category category, DerivedTypeSpec &&spec) {
-  return MakeDerivedType(std::move(spec), category);
-}
-
-DeclTypeSpec &Scope::MakeDerivedType(const Symbol &typeSymbol) {
-  CHECK(typeSymbol.has<DerivedTypeDetails>());
-  CHECK(typeSymbol.scope() != nullptr);
-  return MakeDerivedType(
-      DerivedTypeSpec{typeSymbol}, DeclTypeSpec::TypeDerived);
-}
-
 DeclTypeSpec &Scope::MakeDerivedType(
-    DerivedTypeSpec &&spec, DeclTypeSpec::Category category) {
-  return declTypeSpecs_.emplace_back(
-      category, DerivedTypeSpec{std::move(spec)});
+    DeclTypeSpec::Category category, DerivedTypeSpec &&spec) {
+  return declTypeSpecs_.emplace_back(category, std::move(spec));
 }
 
 void Scope::set_chars(parser::CookedSource &cooked) {

flang/lib/semantics/scope.h

@@ -171,10 +171,7 @@ public:
   const DeclTypeSpec &MakeLogicalType(KindExpr &&kind);
   const DeclTypeSpec &MakeCharacterType(
       ParamValue &&length, KindExpr &&kind = KindExpr{0});
-  const DeclTypeSpec &MakeDerivedType(
-      DeclTypeSpec::Category, DerivedTypeSpec &&);
-  DeclTypeSpec &MakeDerivedType(const Symbol &);
-  DeclTypeSpec &MakeDerivedType(DerivedTypeSpec &&, DeclTypeSpec::Category);
+  DeclTypeSpec &MakeDerivedType(DeclTypeSpec::Category, DerivedTypeSpec &&);
   const DeclTypeSpec &MakeTypeStarType();
   const DeclTypeSpec &MakeClassStarType();
 

flang/lib/semantics/tools.cc

@@ -708,7 +708,7 @@ const DeclTypeSpec &FindOrInstantiateDerivedType(Scope &scope,
   }
   // Create a new instantiation of this parameterized derived type
   // for this particular distinct set of actual parameter values.
-  DeclTypeSpec &type{scope.MakeDerivedType(std::move(spec), category)};
+  DeclTypeSpec &type{scope.MakeDerivedType(category, std::move(spec))};
   InstantiateDerivedType(type.derivedTypeSpec(), scope, semanticsContext);
   return type;
 }

flang/test/semantics/modfile03.f90

@@ -1,4 +1,4 @@
-! Copyright (c) 2018, NVIDIA CORPORATION.  All rights reserved.
+! Copyright (c) 2018-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.
@@ -18,39 +18,35 @@ module m1
   integer :: x1
   integer, private :: x2
 end
-
-module m2
-  use m1
-  integer :: y1
-end
-
-module m3
-  use m2, z1 => x1
-end
-
-module m4
-  use m1
-  use m2
-end
-
 !Expect: m1.mod
 !module m1
 !integer(4)::x1
 !integer(4),private::x2
 !end
 
+module m2
+  use m1
+  integer :: y1
+end
 !Expect: m2.mod
 !module m2
 !use m1,only:x1
 !integer(4)::y1
 !end
 
+module m3
+  use m2, z1 => x1
+end
 !Expect: m3.mod
 !module m3
 !use m2,only:y1
 !use m2,only:z1=>x1
 !end
 
+module m4
+  use m1
+  use m2
+end
 !Expect: m4.mod
 !module m4
 !use m1,only:x1