[flang] Remove some needless operations in expr rewriting

[klausler]

Summary

Expressions emitted to module files and error messages
sometimes contain conversions of integer results of inquiry
intrinsics; these are usually not needed, and can conflict
with "int" in the user's namespace. Improve folding so that
these conversions don't appear, and do some other clean-up
in adjacent code.

[PeteSteinfeld]

All builds, tests, and looks good.

flang/lib/Evaluate/fold-implementation.h

struct {
     Convert<TO, FROMCAT> &convert;
   } msvcWorkaround{context, convert};
   return std::visit(
       [&msvcWorkaround](auto &kindExpr) -> Expr<TO> {
         using Operand = ResultType<decltype(kindExpr)>;
         // This variable is a workaround for msvc which emits an error when
         // using the FROMCAT template parameter below.
         TypeCategory constexpr FromCat{FROMCAT};
+        static_assert(FromCat == Operand::category);
         auto &convert{msvcWorkaround.convert};
         char buffer[64];
         if (auto value{GetScalarConstantValue<Operand>(kindExpr)}) {
           FoldingContext &ctx{msvcWorkaround.context};
           if constexpr (TO::category == TypeCategory::Integer) {
-            if constexpr (Operand::category == TypeCategory::Integer) {
+            if constexpr (FromCat == TypeCategory::Integer) {
               auto converted{Scalar<TO>::ConvertSigned(*value)};
               if (converted.overflow) {
                 ctx.messages().Say(
                     "INTEGER(%d) to INTEGER(%d) conversion overflowed"_en_US,
                     Operand::kind, TO::kind);
               }
               return ScalarConstantToExpr(std::move(converted.value));
-            } else if constexpr (Operand::category == TypeCategory::Real) {
+            } else if constexpr (FromCat == TypeCategory::Real) {
               auto converted{value->template ToInteger<Scalar<TO>>()};
               if (converted.flags.test(RealFlag::InvalidArgument)) {
                 ctx.messages().Say(
                     "REAL(%d) to INTEGER(%d) conversion: invalid argument"_en_US,
                     Operand::kind, TO::kind);
               } else if (converted.flags.test(RealFlag::Overflow)) {
                 ctx.messages().Say(
                     "REAL(%d) to INTEGER(%d) conversion overflowed"_en_US,
                     Operand::kind, TO::kind);
               }
               return ScalarConstantToExpr(std::move(converted.value));
             }
           } else if constexpr (TO::category == TypeCategory::Real) {
-            if constexpr (Operand::category == TypeCategory::Integer) {
+            if constexpr (FromCat == TypeCategory::Integer) {
               auto converted{Scalar<TO>::FromInteger(*value)};
               if (!converted.flags.empty()) {
                 std::snprintf(buffer, sizeof buffer,
                     "INTEGER(%d) to REAL(%d) conversion", Operand::kind,
                     TO::kind);
                 RealFlagWarnings(ctx, converted.flags, buffer);
               }
               return ScalarConstantToExpr(std::move(converted.value));
-            } else if constexpr (Operand::category == TypeCategory::Real) {
+            } else if constexpr (FromCat == TypeCategory::Real) {
               auto converted{Scalar<TO>::Convert(*value)};
               if (!converted.flags.empty()) {
                 std::snprintf(buffer, sizeof buffer,
                     "REAL(%d) to REAL(%d) conversion", Operand::kind, TO::kind);
                 RealFlagWarnings(ctx, converted.flags, buffer);
               }
               if (ctx.flushSubnormalsToZero()) {
                 converted.value = converted.value.FlushSubnormalToZero();
               }
               return ScalarConstantToExpr(std::move(converted.value));
             }
           } else if constexpr (TO::category == TypeCategory::Complex) {
-            if constexpr (Operand::category == TypeCategory::Complex) {
+            if constexpr (FromCat == TypeCategory::Complex) {
               return FoldOperation(ctx,
                   ComplexConstructor<TO::kind>{
                       AsExpr(Convert<typename TO::Part>{AsCategoryExpr(
                           Constant<typename Operand::Part>{value->REAL()})}),
                       AsExpr(Convert<typename TO::Part>{AsCategoryExpr(
                           Constant<typename Operand::Part>{value->AIMAG()})})});
             }
           } else if constexpr (TO::category == TypeCategory::Character &&
-              Operand::category == TypeCategory::Character) {
+              FromCat == TypeCategory::Character) {
             if (auto converted{ConvertString<Scalar<TO>>(std::move(*value))}) {
               return ScalarConstantToExpr(std::move(*converted));
             }
           } else if constexpr (TO::category == TypeCategory::Logical &&
-              Operand::category == TypeCategory::Logical) {
+              FromCat == TypeCategory::Logical) {
             return Expr<TO>{value->IsTrue()};
           }
-        } else if constexpr (std::is_same_v<Operand, TO> &&
+        } else if constexpr (TO::category == FromCat &&
             FromCat != TypeCategory::Character) {
+          // Conversion of non-constant in same type category
+          if constexpr (std::is_same_v<Operand, TO>) {
             return std::move(kindExpr); // remove needless conversion
+          } else if constexpr (std::is_same_v<TO, DescriptorInquiry::Result>) {
+            if (auto *innerConv{
+                    std::get_if<Convert<Operand, TypeCategory::Integer>>(
+                        &kindExpr.u)}) {
+              if (auto *x{std::get_if<Expr<TO>>(&innerConv->left().u)}) {
+                if (std::holds_alternative<DescriptorInquiry>(x->u)) {
+                  // int(int(size(...),kind=k),kind=8) -> size(...)
+                  return std::move(*x);
+                }
+              }
+            }
+          }
         }
         return Expr<TO>{std::move(convert)};
       },
       convert.left().u);
 }
 
 template <typename T>
 Expr<T> FoldOperation(FoldingContext &context, Parentheses<T> &&x) {

 }
 
 template <typename T>
 Expr<T> FoldOperation(FoldingContext &context, Negate<T> &&x) {
   if (auto array{ApplyElementwise(context, x)}) {
     return *array;
   }
   auto &operand{x.left()};
-  if (auto value{GetScalarConstantValue<T>(operand)}) {
+  if (auto *nn{std::get_if<Negate<T>>(&x.left().u)}) {
+    return std::move(nn->left()); // -(-x) -> x
+  } else if (auto value{GetScalarConstantValue<T>(operand)}) {
     if constexpr (T::category == TypeCategory::Integer) {
       auto negated{value->Negate()};
       if (negated.overflow) {
         context.messages().Say(
             "INTEGER(%d) negation overflowed"_en_US, T::kind);
       }
       return Expr<T>{Constant<T>{std::move(negated.value)}};
     } else {
if (auto folded{OperandsAreConstants(x)}) {
     } else {
       auto product{folded->first.Multiply(folded->second, context.rounding())};
       RealFlagWarnings(context, product.flags, "multiplication");
       if (context.flushSubnormalsToZero()) {
         product.value = product.value.FlushSubnormalToZero();
       }
       return Expr<T>{Constant<T>{product.value}};
     }
+  } else if constexpr (T::category == TypeCategory::Integer) {
+    if (auto c{GetScalarConstantValue<T>(x.right())}) {
+      x.right() = std::move(x.left());
+      x.left() = Expr<T>{std::move(*c)};
+    }
+    if (auto c{GetScalarConstantValue<T>(x.left())}) {
+      if (c->IsZero()) {
+        return std::move(x.left());
+      } else if (c->CompareSigned(Scalar<T>{1}) == Ordering::Equal) {
+        return std::move(x.right());
+      } else if (c->CompareSigned(Scalar<T>{-1}) == Ordering::Equal) {
+        return Expr<T>{Negate<T>{std::move(x.right())}};
+      }
+    }
   }
   return Expr<T>{std::move(x)};
 }
 
 template <typename T>
 Expr<T> FoldOperation(FoldingContext &context, Divide<T> &&x) {
   if (auto array{ApplyElementwise(context, x)}) {
     return *array;

flang/test/Semantics/modfile17.f90

 !integer(8),kind::k8
 !integer(int(int(k1,kind=1),kind=8))::j1
 !integer(int(int(k2,kind=2),kind=8))::j2
 !integer(int(int(k4,kind=4),kind=8))::j4
 !integer(k8)::j8
 !end type
 !type::defaulted(n1,n2,n4,n8)
 !integer(1),kind::n1=1_1
-!integer(2),kind::n2=int(int(int(n1,kind=1),kind=4)*2_4,kind=2)
+!integer(2),kind::n2=int(2_4*int(int(n1,kind=1),kind=4),kind=2)
 !integer(4),kind::n4=2_4*int(int(n2,kind=2),kind=4)
 !integer(8),kind::n8=int(12_4-int(n4,kind=4),kind=8)
 !type(capture(k1=int(n1,kind=1),k2=int(n2,kind=2),k4=int(n4,kind=4),k8=n8))::cap
 !end type
 !type,extends(defaulted)::extension(k5)
 !integer(4),kind::k5=4_4
 !integer(int(int(k5,kind=4),kind=8))::j5
 !end type

flang/test/Semantics/modfile30.f90

contains
   end
 end
 
 !Expect: m1.mod
 !module m1
 !contains
 ! function f1(x) result(y)
 !  integer(4)::x(:)
-!  integer(4)::y(1_8:int(int(1_8*size(x,dim=1),kind=4),kind=8))
+!  integer(4)::y(1_8:size(x,dim=1))
 ! end
 ! function f2(x)
 !  integer(4)::x(:)
-!  integer(4)::f2(1_8:int(int(1_8*size(x,dim=1),kind=4),kind=8))
+!  integer(4)::f2(1_8:size(x,dim=1))
 ! end
 !end
 
 ! Order of names in PUBLIC statement shouldn't affect .mod file.
 module m2
   public :: a
   type t
   end type