From 4c4f4d280756daa12d12aae534ab842b81e63789 Mon Sep 17 00:00:00 2001
From: Tim Keith <tkeith@nvidia.com>
Date: Thu, 9 May 2019 10:00:13 -0700
Subject: [PATCH] [flang] Address review comments

Use better names for template parameters of `LoopBounds`, and also
for the constructor arguments.

Use two overloadings of `loopBounds()` instead of `std::is_same_v<>`.

Original-commit: flang-compiler/f18@a2c0c75462caad57af40aa46a58131e946e21ce7
Reviewed-on: https://github.com/flang-compiler/f18/pull/455
Tree-same-pre-rewrite: false
---
 flang/lib/parser/grammar.h    | 14 +++++++-------
 flang/lib/parser/parse-tree.h | 15 ++++++++-------
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/flang/lib/parser/grammar.h b/flang/lib/parser/grammar.h
index 3a23c2f18a34..7852112cb3c2 100644
--- a/flang/lib/parser/grammar.h
+++ b/flang/lib/parser/grammar.h
@@ -962,14 +962,14 @@ constexpr auto doVariable{scalar(integer(name))};
 
 // NOTE: In loop-control we allow REAL name and bounds too.
 // This means parse them without the integer constraint and check later.
+
+inline constexpr auto loopBounds(decltype(scalarExpr) &p) {
+  return construct<LoopBounds<ScalarName, ScalarExpr>>(
+      scalar(name) / "=", p / ",", p, maybe("," >> p));
+}
 template<typename PA> inline constexpr auto loopBounds(const PA &p) {
-  if constexpr (std::is_same_v<typename PA::resultType, ScalarExpr>) {
-    return construct<LoopBounds<ScalarName, typename PA::resultType>>(
-        scalar(name) / "=", p / ",", p, maybe("," >> p));
-  } else {
-    return construct<LoopBounds<DoVariable, typename PA::resultType>>(
-        doVariable / "=", p / ",", p, maybe("," >> p));
-  }
+  return construct<LoopBounds<DoVariable, typename PA::resultType>>(
+      doVariable / "=", p / ",", p, maybe("," >> p));
 }
 
 // R769 array-constructor -> (/ ac-spec /) | lbracket ac-spec rbracket
diff --git a/flang/lib/parser/parse-tree.h b/flang/lib/parser/parse-tree.h
index 5ea32a83d8f1..5e31144fcd5e 100644
--- a/flang/lib/parser/parse-tree.h
+++ b/flang/lib/parser/parse-tree.h
@@ -1208,15 +1208,16 @@ WRAPPER_CLASS(ArrayConstructor, AcSpec);
 // R1124 do-variable -> scalar-int-variable-name
 using DoVariable = Scalar<Integer<Name>>;
 
-template<typename A, typename B> struct LoopBounds {
+template<typename VAR, typename BOUND> struct LoopBounds {
   LoopBounds(LoopBounds &&that) = default;
-  LoopBounds(A &&n, B &&a, B &&z, std::optional<B> &&s)
-    : name{std::move(n)}, lower{std::move(a)}, upper{std::move(z)},
-      step{std::move(s)} {}
+  LoopBounds(
+      VAR &&name, BOUND &&lower, BOUND &&upper, std::optional<BOUND> &&step)
+    : name{std::move(name)}, lower{std::move(lower)}, upper{std::move(upper)},
+      step{std::move(step)} {}
   LoopBounds &operator=(LoopBounds &&) = default;
-  A name;
-  B lower, upper;
-  std::optional<B> step;
+  VAR name;
+  BOUND lower, upper;
+  std::optional<BOUND> step;
 };
 
 using ScalarName = Scalar<Name>;