Fix Klausler reported review comments

Also rebase, as the branch was quite a way behind. Small conflict
was resolved.

Author: Leporacanthicus

flang/include/flang/Semantics/symbol.h

@@ -715,11 +715,11 @@ class UserReductionDetails {
 
   UserReductionDetails() = default;
 
-  void AddType(const DeclTypeSpec *type) { typeList_.push_back(type); }
+  void AddType(const DeclTypeSpec &type) { typeList_.push_back(&type); }
   const TypeVector &GetTypeList() const { return typeList_; }
 
-  bool SupportsType(const DeclTypeSpec *type) const {
-    return llvm::is_contained(typeList_, type);
+  bool SupportsType(const DeclTypeSpec &type) const {
+    return llvm::is_contained(typeList_, &type);
   }
 
   void AddDecl(const DeclInfo &decl) { declList_.push_back(decl); }

flang/lib/Semantics/check-omp-structure.cpp

@@ -3404,8 +3404,7 @@ bool OmpStructureChecker::CheckReductionOperator(
       valid =
           llvm::is_contained({"max", "min", "iand", "ior", "ieor"}, realName);
       if (!valid) {
-        auto *reductionDetails{name->symbol->detailsIf<UserReductionDetails>()};
-        valid = reductionDetails != nullptr;
+        valid = name->symbol->detailsIf<UserReductionDetails>();
       }
     }
     if (!valid) {
@@ -3534,7 +3533,7 @@ static bool IsReductionAllowedForType(
         const auto *reductionDetails{symbol->detailsIf<UserReductionDetails>()};
         assert(reductionDetails && "Expected to find reductiondetails");
 
-        return reductionDetails->SupportsType(&type);
+        return reductionDetails->SupportsType(type);
       }
       return false;
     }
@@ -3571,7 +3570,7 @@ static bool IsReductionAllowedForType(
       // supported.
       if (const auto *reductionDetails{
               name->symbol->detailsIf<UserReductionDetails>()}) {
-        return reductionDetails->SupportsType(&type);
+        return reductionDetails->SupportsType(type);
       }
 
       // We also need to check for mangled names (max, min, iand, ieor and ior)
@@ -3580,7 +3579,7 @@ static bool IsReductionAllowedForType(
       if (const auto &symbol{scope.FindSymbol(mangledName)}) {
         if (const auto *reductionDetails{
                 symbol->detailsIf<UserReductionDetails>()}) {
-          return reductionDetails->SupportsType(&type);
+          return reductionDetails->SupportsType(type);
         }
       }
     }

flang/lib/Semantics/mod-file.cpp

@@ -1039,20 +1039,23 @@ void ModFileWriter::PutTypeParam(llvm::raw_ostream &os, const Symbol &symbol) {
 
 void ModFileWriter::PutUserReduction(
     llvm::raw_ostream &os, const Symbol &symbol) {
-  auto &details{symbol.get<UserReductionDetails>()};
+  const auto &details{symbol.get<UserReductionDetails>()};
   // The module content for a OpenMP Declare Reduction is the OpenMP
   // declaration. There may be multiple declarations.
   // Decls are pointers, so do not use a referene.
   for (const auto decl : details.GetDeclList()) {
-    if (auto d = std::get_if<const parser::OpenMPDeclareReductionConstruct *>(
-            &decl)) {
-      Unparse(os, **d);
-    } else if (auto s = std::get_if<const parser::OmpMetadirectiveDirective *>(
-                   &decl)) {
-      Unparse(os, **s);
-    } else {
-      DIE("Unknown OpenMP DECLARE REDUCTION content");
-    }
+    common::visit( //
+        common::visitors{//
+            [&](const parser::OpenMPDeclareReductionConstruct *d) {
+              Unparse(os, *d);
+            },
+            [&](const parser::OmpMetadirectiveDirective *m) {
+              Unparse(os, *m);
+            },
+            [&](const auto &) {
+              DIE("Unknown OpenMP DECLARE REDUCTION content");
+            }},
+        decl);
   }
 }
 

flang/lib/Semantics/resolve-names-utils.h

@@ -149,7 +149,7 @@ void MapSubprogramToNewSymbols(const Symbol &oldSymbol, Symbol &newSymbol,
 parser::CharBlock MakeNameFromOperator(
     const parser::DefinedOperator::IntrinsicOperator &op,
     SemanticsContext &context);
-parser::CharBlock MangleSpecialFunctions(const parser::CharBlock name);
+parser::CharBlock MangleSpecialFunctions(const parser::CharBlock &name);
 
 } // namespace Fortran::semantics
 #endif // FORTRAN_SEMANTICS_RESOLVE_NAMES_H_

flang/lib/Semantics/resolve-names.cpp

@@ -1442,11 +1442,15 @@ class OmpVisitor : public virtual DeclarationVisitor {
   static bool NeedsScope(const parser::OpenMPBlockConstruct &);
   static bool NeedsScope(const parser::OmpClause &);
 
-  bool Pre(const parser::OmpMetadirectiveDirective &) {
+  bool Pre(const parser::OmpMetadirectiveDirective &x) { //
+    metaDirective_ = &x;
     ++metaLevel_;
     return true;
   }
-  void Post(const parser::OmpMetadirectiveDirective &) { --metaLevel_; }
+  void Post(const parser::OmpMetadirectiveDirective &) { //
+    metaDirective_ = nullptr;
+    --metaLevel_;
+  }
 
   bool Pre(const parser::OpenMPRequiresConstruct &x) {
     AddOmpSourceRange(x.source);
@@ -1655,14 +1659,6 @@ class OmpVisitor : public virtual DeclarationVisitor {
     EndDeclTypeSpec();
   }
 
-  bool Pre(const parser::OmpMetadirectiveDirective &x) { //
-    metaDirective_ = &x;
-    return true;
-  }
-  void Post(const parser::OmpMetadirectiveDirective &) { //
-    metaDirective_ = nullptr;
-  }
-
 private:
   void ProcessMapperSpecifier(const parser::OmpMapperSpecifier &spec,
       const parser::OmpClauseList &clauses);
@@ -1793,7 +1789,7 @@ parser::CharBlock MakeNameFromOperator(
   }
 }
 
-parser::CharBlock MangleSpecialFunctions(const parser::CharBlock name) {
+parser::CharBlock MangleSpecialFunctions(const parser::CharBlock &name) {
   return llvm::StringSwitch<parser::CharBlock>(name.ToString())
       .Case("max", {"op.max", 6})
       .Case("min", {"op.min", 6})
@@ -1888,7 +1884,7 @@ void OmpVisitor::ProcessReductionSpecifier(
     // Only process types we can find. There will be an error later on when
     // a type isn't found.
     if (const DeclTypeSpec * typeSpec{GetDeclTypeSpec()}) {
-      reductionDetails->AddType(typeSpec);
+      reductionDetails->AddType(*typeSpec);
 
       for (auto &nm : ompVarNames) {
         ObjectEntityDetails details{};