[flang] Reorganize evaluate::Assignment
authorTim Keith <tkeith@nvidia.com>
Tue, 18 Feb 2020 23:20:28 +0000 (15:20 -0800)
committerTim Keith <tkeith@nvidia.com>
Tue, 18 Feb 2020 23:20:28 +0000 (15:20 -0800)
Every analyzed assignment represented by `evaluate::Assignment` has
a LHS and RHS expression. These need to be checked uniformly in various
places. So change Assignment always to have those data members, with
the variant determining which kinds of assignment it is: intrinsic,
user-defined, or pointer.

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

flang/include/flang/evaluate/expression.h
flang/lib/evaluate/expression.cpp
flang/lib/semantics/assignment.cpp
flang/lib/semantics/expression.cpp
flang/lib/semantics/pointer-assignment.cpp
flang/lib/semantics/pointer-assignment.h

index b061191..019858a 100644 (file)
@@ -811,28 +811,24 @@ public:
   common::CombineVariants<TypelessExpression, CategoryExpression> u;
 };
 
-// An assignment is either intrinsic (with lhs and rhs) or user-defined,
-// represented as a ProcedureRef. A pointer assignment optionally also has
-// a bounds-spec or bounds-remapping.
+// An assignment is either intrinsic, user-defined (with a ProcedureRef to
+// specify the procedure to call), or pointer assignment (with possibly empty
+// BoundsSpec or non-empty BoundsRemapping). In all cases there are Exprs
+// representing the LHS and RHS of the assignment.
 class Assignment {
 public:
-  UNION_CONSTRUCTORS(Assignment)
-  struct IntrinsicAssignment {
-    Expr<SomeType> lhs;
-    Expr<SomeType> rhs;
-  };
-  struct PointerAssignment {
-    using BoundsSpec = std::vector<Expr<SubscriptInteger>>;
-    using BoundsRemapping =
-        std::vector<std::pair<Expr<SubscriptInteger>, Expr<SubscriptInteger>>>;
-    PointerAssignment(Expr<SomeType> &&lhs, Expr<SomeType> &&rhs)
-      : lhs{std::move(lhs)}, rhs{std::move(rhs)} {}
-    Expr<SomeType> lhs;
-    Expr<SomeType> rhs;
-    std::variant<BoundsSpec, BoundsRemapping> bounds;
-  };
+  Assignment(Expr<SomeType> &&lhs, Expr<SomeType> &&rhs)
+    : lhs(std::move(lhs)), rhs(std::move(rhs)) {}
+
+  struct Intrinsic {};
+  using BoundsSpec = std::vector<Expr<SubscriptInteger>>;
+  using BoundsRemapping =
+      std::vector<std::pair<Expr<SubscriptInteger>, Expr<SubscriptInteger>>>;
   std::ostream &AsFortran(std::ostream &) const;
-  std::variant<IntrinsicAssignment, ProcedureRef, PointerAssignment> u;
+
+  Expr<SomeType> lhs;
+  Expr<SomeType> rhs;
+  std::variant<Intrinsic, ProcedureRef, BoundsSpec, BoundsRemapping> u;
 };
 
 // This wrapper class is used, by means of a forward reference with
index fbf20da..c805996 100644 (file)
@@ -168,40 +168,33 @@ GenericExprWrapper::~GenericExprWrapper() {}
 std::ostream &Assignment::AsFortran(std::ostream &o) const {
   std::visit(
       common::visitors{
-          [&](const evaluate::Assignment::IntrinsicAssignment &x) {
-            x.rhs.AsFortran(x.lhs.AsFortran(o) << '=');
+          [&](const Assignment::Intrinsic &) {
+            rhs.AsFortran(lhs.AsFortran(o) << '=');
           },
-          [&](const evaluate::ProcedureRef &x) { x.AsFortran(o << "CALL "); },
-          [&](const evaluate::Assignment::PointerAssignment &x) {
-            x.lhs.AsFortran(o);
-            std::visit(
-                common::visitors{
-                    [&](const evaluate::Assignment::PointerAssignment::
-                            BoundsSpec &bounds) {
-                      if (!bounds.empty()) {
-                        char sep{'('};
-                        for (const auto &bound : bounds) {
-                          bound.AsFortran(o << sep) << ':';
-                          sep = ',';
-                        }
-                        o << ')';
-                      }
-                    },
-                    [&](const evaluate::Assignment::PointerAssignment::
-                            BoundsRemapping &bounds) {
-                      if (!bounds.empty()) {
-                        char sep{'('};
-                        for (const auto &bound : bounds) {
-                          bound.first.AsFortran(o << sep) << ':';
-                          bound.second.AsFortran(o);
-                          sep = ',';
-                        }
-                        o << ')';
-                      }
-                    },
-                },
-                x.bounds);
-            x.rhs.AsFortran(o << " => ");
+          [&](const ProcedureRef &proc) { proc.AsFortran(o << "CALL "); },
+          [&](const BoundsSpec &bounds) {
+            lhs.AsFortran(o);
+            if (!bounds.empty()) {
+              char sep{'('};
+              for (const auto &bound : bounds) {
+                bound.AsFortran(o << sep) << ':';
+                sep = ',';
+              }
+              o << ')';
+            }
+          },
+          [&](const BoundsRemapping &bounds) {
+            lhs.AsFortran(o);
+            if (!bounds.empty()) {
+              char sep{'('};
+              for (const auto &bound : bounds) {
+                bound.first.AsFortran(o << sep) << ':';
+                bound.second.AsFortran(o);
+                sep = ',';
+              }
+              o << ')';
+            }
+            rhs.AsFortran(o << " => ");
           },
       },
       u);
index 362df04..cb727ef 100644 (file)
@@ -141,50 +141,44 @@ private:
 void AssignmentContext::Analyze(const parser::AssignmentStmt &stmt) {
   // Assignment statement analysis is in expression.cpp where user-defined
   // assignments can be recognized and replaced.
-  if (const evaluate::Assignment * asst{GetAssignment(stmt)}) {
-    if (const auto *intrinsicAsst{
-            std::get_if<evaluate::Assignment::IntrinsicAssignment>(&asst->u)}) {
-      CheckForImpureCall(intrinsicAsst->lhs);
-      CheckForImpureCall(intrinsicAsst->rhs);
-      if (forall_) {
-        // TODO: Warn if some name in forall_->activeNames or its outer
-        // contexts does not appear on LHS
-      }
-      CheckForPureContext(intrinsicAsst->lhs, intrinsicAsst->rhs,
-          std::get<parser::Expr>(stmt.t).source, false /* not => */);
+  if (const evaluate::Assignment * assignment{GetAssignment(stmt)}) {
+    CheckForImpureCall(assignment->lhs);
+    CheckForImpureCall(assignment->rhs);
+    if (forall_) {
+      // TODO: Warn if some name in forall_->activeNames or its outer
+      // contexts does not appear on LHS
     }
+    CheckForPureContext(assignment->lhs, assignment->rhs,
+        std::get<parser::Expr>(stmt.t).source, false /* not => */);
   }
   // TODO: Fortran 2003 ALLOCATABLE assignment semantics (automatic
   // (re)allocation of LHS array when unallocated or nonconformable)
 }
 
 void AssignmentContext::Analyze(const parser::PointerAssignmentStmt &stmt) {
-  using PointerAssignment = evaluate::Assignment::PointerAssignment;
   CHECK(!where_);
-  const evaluate::Assignment *assign{GetAssignment(stmt)};
-  if (!assign) {
+  const evaluate::Assignment *assignment{GetAssignment(stmt)};
+  if (!assignment) {
     return;
   }
-  const auto &ptrAssign{std::get<PointerAssignment>(assign->u)};
-  const SomeExpr &lhs{ptrAssign.lhs};
-  const SomeExpr &rhs{ptrAssign.rhs};
+  const SomeExpr &lhs{assignment->lhs};
+  const SomeExpr &rhs{assignment->rhs};
   CheckForImpureCall(lhs);
   CheckForImpureCall(rhs);
   std::visit(
-      common::visitors{
-          [&](const PointerAssignment::BoundsSpec &bounds) {
-            for (const auto &bound : bounds) {
-              CheckForImpureCall(SomeExpr{bound});
-            }
-          },
-          [&](const PointerAssignment::BoundsRemapping &bounds) {
+      common::visitors{[&](const evaluate::Assignment::BoundsSpec &bounds) {
+                         for (const auto &bound : bounds) {
+                           CheckForImpureCall(SomeExpr{bound});
+                         }
+                       },
+          [&](const evaluate::Assignment::BoundsRemapping &bounds) {
             for (const auto &bound : bounds) {
               CheckForImpureCall(SomeExpr{bound.first});
               CheckForImpureCall(SomeExpr{bound.second});
             }
           },
-      },
-      ptrAssign.bounds);
+          [](const auto &) { DIE("not valid for pointer assignment"); }},
+      assignment->u);
   if (forall_) {
     // TODO: Warn if some name in forall_->activeNames or its outer
     // contexts does not appear on LHS
@@ -193,7 +187,7 @@ void AssignmentContext::Analyze(const parser::PointerAssignmentStmt &stmt) {
       true /* isPointerAssignment */);
   auto restorer{context_.foldingContext().messages().SetLocation(
       context_.location().value())};
-  CheckPointerAssignment(context_.foldingContext(), ptrAssign);
+  CheckPointerAssignment(context_.foldingContext(), *assignment);
 }
 
 void AssignmentContext::Analyze(const parser::WhereStmt &stmt) {
index 32e6692..054b4d1 100644 (file)
@@ -1919,10 +1919,13 @@ const Assignment *ExpressionAnalyzer::Analyze(const parser::AssignmentStmt &x) {
       x.typedAssignment.reset(new GenericAssignmentWrapper{});
     } else {
       std::optional<ProcedureRef> procRef{analyzer.TryDefinedAssignment()};
-      x.typedAssignment.reset(new GenericAssignmentWrapper{procRef
-              ? Assignment{std::move(*procRef)}
-              : Assignment{Assignment::IntrinsicAssignment{
-                    Fold(analyzer.MoveExpr(0)), Fold(analyzer.MoveExpr(1))}}});
+      Assignment assignment{
+          Fold(analyzer.MoveExpr(0)), Fold(analyzer.MoveExpr(1))};
+      if (procRef) {
+        assignment.u = std::move(*procRef);
+      }
+      x.typedAssignment.reset(
+          new GenericAssignmentWrapper{std::move(assignment)});
     }
   }
   return common::GetPtrFromOptional(x.typedAssignment->v);
@@ -1933,12 +1936,14 @@ const Assignment *ExpressionAnalyzer::Analyze(
   if (!x.typedAssignment) {
     MaybeExpr lhs{Analyze(std::get<parser::DataRef>(x.t))};
     MaybeExpr rhs{Analyze(std::get<parser::Expr>(x.t))};
-    decltype(Assignment::PointerAssignment::bounds) pointerBounds;
-    std::visit(
-        common::visitors{
-            [&](const std::list<parser::BoundsRemapping> &list) {
-              if (!list.empty()) {
-                Assignment::PointerAssignment::BoundsRemapping bounds;
+    if (!lhs || !rhs) {
+      x.typedAssignment.reset(new GenericAssignmentWrapper{});
+    } else {
+      Assignment assignment{std::move(*lhs), std::move(*rhs)};
+      std::visit(
+          common::visitors{
+              [&](const std::list<parser::BoundsRemapping> &list) {
+                Assignment::BoundsRemapping bounds;
                 for (const auto &elem : list) {
                   auto lower{AsSubscript(Analyze(std::get<0>(elem.t)))};
                   auto upper{AsSubscript(Analyze(std::get<1>(elem.t)))};
@@ -1947,30 +1952,21 @@ const Assignment *ExpressionAnalyzer::Analyze(
                         Fold(std::move(*lower)), Fold(std::move(*upper)));
                   }
                 }
-                pointerBounds = bounds;
-              }
-            },
-            [&](const std::list<parser::BoundsSpec> &list) {
-              if (!list.empty()) {
-                Assignment::PointerAssignment::BoundsSpec bounds;
+                assignment.u = std::move(bounds);
+              },
+              [&](const std::list<parser::BoundsSpec> &list) {
+                Assignment::BoundsSpec bounds;
                 for (const auto &bound : list) {
                   if (auto lower{AsSubscript(Analyze(bound.v))}) {
                     bounds.emplace_back(Fold(std::move(*lower)));
                   }
                 }
-                pointerBounds = bounds;
-              }
-            },
-        },
-        std::get<parser::PointerAssignmentStmt::Bounds>(x.t).u);
-    if (!lhs || !rhs) {
-      x.typedAssignment.reset(new GenericAssignmentWrapper{});
-    } else {
-      Assignment::PointerAssignment assignment{
-          Fold(std::move(*lhs)), Fold(std::move(*rhs))};
-      assignment.bounds = pointerBounds;
+                assignment.u = std::move(bounds);
+              },
+          },
+          std::get<parser::PointerAssignmentStmt::Bounds>(x.t).u);
       x.typedAssignment.reset(
-          new GenericAssignmentWrapper{Assignment{std::move(assignment)}});
+          new GenericAssignmentWrapper{std::move(assignment)});
     }
   }
   return common::GetPtrFromOptional(x.typedAssignment->v);
@@ -2784,8 +2780,9 @@ std::optional<ProcedureRef> ArgumentAnalyzer::GetDefinedAssignmentProc() {
     }
   }
   if (proc) {
-    actuals_[1]->Parenthesize();
-    return ProcedureRef{ProcedureDesignator{*proc}, std::move(actuals_)};
+    ActualArguments actualsCopy{actuals_};
+    actualsCopy[1]->Parenthesize();
+    return ProcedureRef{ProcedureDesignator{*proc}, std::move(actualsCopy)};
   } else {
     return std::nullopt;
   }
index 8e111ef..d3b3ec7 100644 (file)
@@ -35,7 +35,6 @@ using evaluate::characteristics::Procedure;
 using evaluate::characteristics::TypeAndShape;
 using parser::MessageFixedText;
 using parser::MessageFormattedText;
-using PointerAssignment = evaluate::Assignment::PointerAssignment;
 
 class PointerAssignmentChecker {
 public:
@@ -348,17 +347,17 @@ parser::Message *PointerAssignmentChecker::Say(A &&... x) {
 // Verify that any bounds on the LHS of a pointer assignment are valid.
 // Return true if it is a bound-remapping so we can perform further checks.
 static bool CheckPointerBounds(
-    evaluate::FoldingContext &context, const PointerAssignment &assignment) {
+    evaluate::FoldingContext &context, const evaluate::Assignment &assignment) {
   auto &messages{context.messages()};
   const SomeExpr &lhs{assignment.lhs};
   const SomeExpr &rhs{assignment.rhs};
   bool isBoundsRemapping{false};
   std::size_t numBounds{std::visit(
       common::visitors{
-          [&](const PointerAssignment::BoundsSpec &bounds) {
+          [&](const evaluate::Assignment::BoundsSpec &bounds) {
             return bounds.size();
           },
-          [&](const PointerAssignment::BoundsRemapping &bounds) {
+          [&](const evaluate::Assignment::BoundsRemapping &bounds) {
             isBoundsRemapping = true;
             evaluate::ExtentExpr lhsSizeExpr{1};
             for (const auto &bound : bounds) {
@@ -383,8 +382,11 @@ static bool CheckPointerBounds(
             }
             return bounds.size();
           },
+          [](const auto &) -> std::size_t {
+            DIE("not valid for pointer assignment");
+          },
       },
-      assignment.bounds)};
+      assignment.u)};
   if (numBounds > 0) {
     if (lhs.Rank() != static_cast<int>(numBounds)) {
       messages.Say("Pointer '%s' has rank %d but the number of bounds specified"
@@ -401,7 +403,7 @@ static bool CheckPointerBounds(
 }
 
 void CheckPointerAssignment(
-    evaluate::FoldingContext &context, const PointerAssignment &assignment) {
+    evaluate::FoldingContext &context, const evaluate::Assignment &assignment) {
   const SomeExpr &lhs{assignment.lhs};
   const SomeExpr &rhs{assignment.rhs};
   const Symbol *pointer{GetLastSymbol(lhs)};
index 70c8056..a9efc59 100644 (file)
@@ -26,8 +26,8 @@ namespace Fortran::semantics {
 
 class Symbol;
 
-void CheckPointerAssignment(evaluate::FoldingContext &,
-    const evaluate::Assignment::PointerAssignment &);
+void CheckPointerAssignment(
+    evaluate::FoldingContext &, const evaluate::Assignment &);
 void CheckPointerAssignment(
     evaluate::FoldingContext &, const Symbol &lhs, const SomeExpr &rhs);
 void CheckPointerAssignment(evaluate::FoldingContext &,