[flang] some clean-up

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

flang/lib/evaluate/expression.cc

@@ -493,8 +493,9 @@ Expr<SubscriptInteger> Expr<Type<TypeCategory::Character, KIND>>::LEN() const {
                          return AsExpr(Constant<SubscriptInteger>{
                              static_cast<std::uint64_t>(c.value.size())});
                        },
+          [](const Parentheses<Result> &x) { return x.left().LEN(); },
           [](const Concat<KIND> &c) {
-            return c.left().LEN() + c.template right().LEN();
+            return c.left().LEN() + c.right().LEN();
           },
           [](const Extremum<Result> &c) {
             return Expr<SubscriptInteger>{
@@ -514,12 +515,8 @@ auto ExpressionBase<RESULT>::ScalarValue() const
     if (auto *c{std::get_if<Constant<Result>>(&derived().u)}) {
       return {c->value};
     }
-    // TODO: every specifically-typed Expr should support Parentheses
+    if (auto *p{std::get_if<Parentheses<Result>>(&derived().u)}) {
-    if constexpr (common::HasMember<Parentheses<Result>,
+      return p->left().ScalarValue();
-                      decltype(derived().u)>) {
-      if (auto *p{std::get_if<Parentheses<Result>>(&derived().u)}) {
-        return p->left().ScalarValue();
-      }
     }
   } else if constexpr (std::is_same_v<Result, SomeType>) {
     return std::visit(

flang/lib/evaluate/expression.h

@@ -520,8 +520,7 @@ public:
   Expr<SubscriptInteger> LEN() const;
 
   std::variant<Constant<Result>, Designator<Result>, FunctionReference<Result>,
-      // TODO Parentheses<Result>,
+      Parentheses<Result>, Concat<KIND>, Extremum<Result>>
-      Concat<KIND>, Extremum<Result>>
       u;
 };
 
@@ -594,8 +593,9 @@ public:
   explicit Expr(bool x) : u{Constant<Result>{x}} {}
 
 private:
-  using Operations = std::variant<Convert<Result, TypeCategory::Logical>,
+  using Operations =
-      Not<KIND>, LogicalOperation<KIND>, Relational<SomeType>>;
+      std::variant<Convert<Result, TypeCategory::Logical>, Parentheses<Result>,
+          Not<KIND>, LogicalOperation<KIND>, Relational<SomeType>>;
   using Others = std::variant<Constant<Result>, Designator<Result>,
       FunctionReference<Result>>;
 

flang/lib/evaluate/tools.cc

@@ -105,13 +105,13 @@ std::optional<Expr<SomeType>> MixedRealLeft(
       [&](auto &&rxk) -> Expr<SomeReal> {
         using resultType = ResultType<decltype(rxk)>;
         if constexpr (std::is_same_v<OPR<resultType>, Power<resultType>>) {
-          return AsCategoryExpr(AsExpr(
+          return AsCategoryExpr(
-              RealToIntPower<resultType>{std::move(rxk), std::move(iy)}));
+              RealToIntPower<resultType>{std::move(rxk), std::move(iy)});
         }
         // G++ 8.1.0 emits bogus warnings about missing return statements if
         // this statement is wrapped in an "else", as it should be.
-        return AsCategoryExpr(AsExpr(OPR<resultType>{
+        return AsCategoryExpr(OPR<resultType>{
-            std::move(rxk), ConvertToType<resultType>(std::move(iy))}));
+            std::move(rxk), ConvertToType<resultType>(std::move(iy))});
       },
       std::move(rx.u)));
 }
@@ -144,7 +144,7 @@ Expr<SomeReal> GetComplexPart(const Expr<SomeComplex> &z, bool isImaginary) {
   return std::visit(
       [&](const auto &zk) {
         static constexpr int kind{ResultType<decltype(zk)>::kind};
-        return AsCategoryExpr(AsExpr(ComplexComponent<kind>{isImaginary, zk}));
+        return AsCategoryExpr(ComplexComponent<kind>{isImaginary, zk});
       },
       z.u);
 }
@@ -257,9 +257,9 @@ std::optional<Expr<SomeType>> NumericOperation(
             return Package(std::visit(
                 [&](auto &&ryk) -> Expr<SomeReal> {
                   using resultType = ResultType<decltype(ryk)>;
-                  return AsCategoryExpr(AsExpr(
+                  return AsCategoryExpr(
                       OPR<resultType>{ConvertToType<resultType>(std::move(ix)),
-                          std::move(ryk)}));
+                          std::move(ryk)});
                 },
                 std::move(ry.u)));
           },

flang/lib/evaluate/variable.h

@@ -276,6 +276,8 @@ struct ActualFunctionArg {
   explicit ActualFunctionArg(Expr<SomeType> &&x) : u{std::move(x)} {}
   std::ostream &Dump(std::ostream &) const;
 
+  // Subtlety: There is a distinction to be respected here between a variable
+  // and an expression that is a variable, e.g. X vs. (X).
   std::variant<CopyableIndirection<Expr<SomeType>>, Variable> u;
 };
 
@@ -293,13 +295,8 @@ public:
   explicit ActualSubroutineArg(const Label &l) : u{&l} {}
   std::ostream &Dump(std::ostream &) const;
 
-private:
-  using Variables = decltype(Variable::u);
-  using Others =
-      std::variant<CopyableIndirection<Expr<SomeType>>, const Label *>;
-
 public:
-  common::CombineVariants<Variables, Others> u;
+  std::variant<CopyableIndirection<Expr<SomeType>>, Variable, const Label *> u;
 };
 
 using SubroutineRef = ProcedureRef<ActualSubroutineArg>;

flang/lib/semantics/expression.cc

@@ -258,7 +258,7 @@ struct RealTypeVisitor {
   template<std::size_t J> Result Test() {
     using Ty = std::tuple_element_t<J, RealTypes>;
     if (kind == Ty::kind) {
-      return {AsCategoryExpr(AsExpr(ReadRealLiteral<Ty>(literal, context)))};
+      return {AsCategoryExpr(ReadRealLiteral<Ty>(literal, context))};
     }
     return std::nullopt;
   }
@@ -643,8 +643,8 @@ MaybeExpr ExprAnalyzer::Analyze(const parser::StructureComponent &sc) {
         Expr<SomeReal> realExpr{std::visit(
             [&](const auto &z) {
               using PartType = typename ResultType<decltype(z)>::Part;
-              return AsCategoryExpr(AsExpr(Designator<PartType>{
+              return AsCategoryExpr(
-                  ComplexPart{std::move(*dataRef), part}}));
+                  Designator<PartType>{ComplexPart{std::move(*dataRef), part}});
             },
             zExpr->u)};
         return {AsGenericExpr(std::move(realExpr))};
@@ -679,6 +679,8 @@ MaybeExpr ExprAnalyzer::Analyze(const parser::StructureConstructor &) {
 }
 
 MaybeExpr ExprAnalyzer::Analyze(const parser::FunctionReference &) {
+  // TODO: C1003: A parenthesized function reference may not return a
+  // procedure pointer.
   context.messages.Say("TODO: FunctionReference unimplemented"_err_en_US);
   return std::nullopt;
 }
@@ -688,20 +690,17 @@ MaybeExpr ExprAnalyzer::Analyze(const parser::Expr::Parentheses &x) {
     return std::visit(
         common::visitors{
             [&](BOZLiteralConstant &&boz) {
-              return operand;  // ignore parentheses around typeless
+              return operand;  // ignore parentheses around typeless constants
+            },
+            [&](Expr<SomeDerived> &&) {
+              // TODO: parenthesized derived type variable
+              return operand;
             },
-            [&](Expr<SomeDerived> &&dte) { return operand; },
             [](auto &&catExpr) {
               return std::visit(
                   [](auto &&expr) -> MaybeExpr {
                     using Ty = ResultType<decltype(expr)>;
-                    if constexpr (common::HasMember<Parentheses<Ty>,
+                    return {AsGenericExpr(Parentheses<Ty>{std::move(expr)})};
-                                      decltype(expr.u)>) {
-                      return {AsGenericExpr(
-                          AsExpr(Parentheses<Ty>{std::move(expr)}))};
-                    }
-                    // TODO: support Parentheses in all Expr specializations
-                    return std::nullopt;
                   },
                   std::move(catExpr.u));
             }},
@@ -811,8 +810,8 @@ MaybeExpr ExprAnalyzer::Analyze(const parser::Expr::Concat &x) {
                     using Ty = ResultType<decltype(cxk)>;
                     if constexpr (std::is_same_v<Ty,
                                       ResultType<decltype(cyk)>>) {
-                      return {AsGenericExpr(AsCategoryExpr(AsExpr(
+                      return {AsGenericExpr(
-                          Concat<Ty::kind>{std::move(cxk), std::move(cyk)})))};
+                          Concat<Ty::kind>{std::move(cxk), std::move(cyk)})};
                     } else {
                       context.messages.Say(
                           "Operands of // must be the same kind of CHARACTER"_err_en_US);