From ea1cf9ee0b8bb6d464a254659f6badefded0759a Mon Sep 17 00:00:00 2001
From: Pete Steinfeld <psteinfeld@nvidia.com>
Date: Tue, 24 Sep 2019 15:33:51 -0700
Subject: [PATCH] [flang] More responses to pull request comments.

Original-commit: flang-compiler/f18@0fb4cf1c9b3fe36e2c5075981e0d7ce61cb49aae
Reviewed-on: https://github.com/flang-compiler/f18/pull/756
---
 flang/lib/common/template.h     |  2 +-
 flang/lib/semantics/check-do.cc | 30 ++++++++++++++----------------
 2 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/flang/lib/common/template.h b/flang/lib/common/template.h
index 3f8485beec00..b0cbba7721be 100644
--- a/flang/lib/common/template.h
+++ b/flang/lib/common/template.h
@@ -115,7 +115,7 @@ std::optional<A> JoinOptional(std::optional<std::optional<A>> &&x) {
 template<typename A>
 const A *GetPtrFromOptional(const std::optional<A> &x) {
   if (x.has_value()) {
-    return &x.value();
+    return &*x;
   } else {
     return nullptr;
   }
diff --git a/flang/lib/semantics/check-do.cc b/flang/lib/semantics/check-do.cc
index 73928e5c20c6..5b20c0aba6c9 100644
--- a/flang/lib/semantics/check-do.cc
+++ b/flang/lib/semantics/check-do.cc
@@ -28,24 +28,23 @@
 namespace Fortran::semantics {
 
 using namespace parser::literals;
-using namespace common;
 
 // Return the (possibly null)  name of the construct
 template<typename A>
 static const parser::Name *MaybeGetConstructName(const A &a) {
-  return GetPtrFromOptional(std::get<0>(std::get<0>(a.t).statement.t));
+  return common::GetPtrFromOptional(std::get<0>(std::get<0>(a.t).statement.t));
 }
 
 static const parser::Name *MaybeGetConstructName(
     const parser::BlockConstruct &blockConstruct) {
-  return GetPtrFromOptional(
+  return common::GetPtrFromOptional(
       std::get<parser::Statement<parser::BlockStmt>>(blockConstruct.t)
           .statement.v);
 }
 
 // Return the (possibly null) name of the statement
 template<typename A> static const parser::Name *MaybeGetStmtName(const A &a) {
-  return GetPtrFromOptional(std::get<0>(a.t));
+  return common::GetPtrFromOptional(std::get<0>(a.t));
 }
 
 // 11.1.7.5 - enforce semantics constraints on a DO CONCURRENT loop body
@@ -668,15 +667,16 @@ void DoChecker::SayBadLeave(StmtType stmtType, const char *enclosingStmtName,
 
 static const parser::DoConstruct *MaybeGetDoConstruct(
     const ConstructNode &construct) {
-  if (auto const *doNode{
+  if (const auto *doNode{
           std::get_if<const parser::DoConstruct *>(&construct)}) {
     return *doNode;
+  } else {
+    return nullptr;
   }
-  return nullptr;
 }
 
 static bool ConstructIsDoConcurrent(const ConstructNode &construct) {
-  parser::DoConstruct const *doConstruct{MaybeGetDoConstruct(construct)};
+  const parser::DoConstruct *doConstruct{MaybeGetDoConstruct(construct)};
   return doConstruct && doConstruct->IsDoConcurrent();
 }
 
@@ -705,8 +705,6 @@ void DoChecker::CheckForBadLeave(
       construct);
 }
 
-static bool isExit(StmtType stmtType) { return stmtType == StmtType::EXIT; }
-
 static bool StmtMatchesConstruct(const parser::Name *stmtName,
     StmtType stmtType, const parser::Name *constructName,
     const ConstructNode &construct) {
@@ -714,7 +712,7 @@ static bool StmtMatchesConstruct(const parser::Name *stmtName,
   if (stmtName == nullptr) {
     return inDoConstruct;  // Unlabeled statements match all DO constructs
   } else if (constructName && constructName->source == stmtName->source) {
-    return isExit(stmtType) || inDoConstruct;
+    return stmtType == StmtType::EXIT || inDoConstruct;
   } else {
     return false;
   }
@@ -723,7 +721,7 @@ static bool StmtMatchesConstruct(const parser::Name *stmtName,
 // C1167 Can't EXIT from a DO CONCURRENT
 void DoChecker::CheckDoConcurrentExit(
     StmtType stmtType, const ConstructNode &construct) const {
-  if (isExit(stmtType) && ConstructIsDoConcurrent(construct)) {
+  if (stmtType == StmtType::EXIT && ConstructIsDoConcurrent(construct)) {
     SayBadLeave(StmtType::EXIT, "DO CONCURRENT", construct);
   }
 }
@@ -735,8 +733,8 @@ void DoChecker::CheckDoConcurrentExit(
 void DoChecker::CheckNesting(
     StmtType stmtType, const parser::Name *stmtName) const {
   const ConstructStack &stack{context_.constructStack()};
-  for (auto rIter{stack.crbegin()}; rIter != stack.crend(); ++rIter) {
-    const ConstructNode &construct{*rIter};
+  for (auto iter{stack.cend()}; iter-- != stack.cbegin();) {
+    const ConstructNode &construct{*iter};
     const parser::Name *constructName{MaybeGetNodeName(construct)};
     if (StmtMatchesConstruct(stmtName, stmtType, constructName, construct)) {
       CheckDoConcurrentExit(stmtType, construct);
@@ -746,7 +744,7 @@ void DoChecker::CheckNesting(
   }
 
   // We haven't found a match in the enclosing constructs
-  if (isExit(stmtType)) {
+  if (stmtType == StmtType::EXIT) {
     context_.Say("No matching construct for EXIT statement"_err_en_US);
   } else {
     context_.Say("No matching DO construct for CYCLE statement"_err_en_US);
@@ -755,12 +753,12 @@ void DoChecker::CheckNesting(
 
 // C1135
 void DoChecker::Enter(const parser::CycleStmt &cycleStmt) {
-  CheckNesting(StmtType::CYCLE, GetPtrFromOptional(cycleStmt.v));
+  CheckNesting(StmtType::CYCLE, common::GetPtrFromOptional(cycleStmt.v));
 }
 
 // C1167 and C1168
 void DoChecker::Enter(const parser::ExitStmt &exitStmt) {
-  CheckNesting(StmtType::EXIT, GetPtrFromOptional(exitStmt.v));
+  CheckNesting(StmtType::EXIT, common::GetPtrFromOptional(exitStmt.v));
 }
 
 }  // namespace Fortran::semantics