From 510671c8c1d1d93c39fab0edd3e1f8c14af4711d Mon Sep 17 00:00:00 2001
From: Tim Keith <tkeith@nvidia.com>
Date: Fri, 19 Apr 2019 17:14:50 -0700
Subject: [PATCH] [flang] Distinguish between unanalyzed and error expressions

When an Expr or Variable is analyzed, always fill in a GenericExprWrapper
for it. That now holds the result of expression analysis which is
std::nullopt when there is an error.

After the ExprChecker pass has finished, the typedExpr data member of
Variables and top-level Exprs should be filled in. Assert that is the
case when we access them.

Original-commit: flang-compiler/f18@192a4e08554b105c48fcd65e6f8ed4860e64f544
Reviewed-on: https://github.com/flang-compiler/f18/pull/424
Tree-same-pre-rewrite: false
---
 flang/lib/FIR/afforestation.cc    |  8 ++++++--
 flang/lib/evaluate/expression.h   |  5 +++--
 flang/lib/semantics/expression.cc | 12 ++++--------
 flang/lib/semantics/tools.h       |  3 ++-
 4 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/flang/lib/FIR/afforestation.cc b/flang/lib/FIR/afforestation.cc
index 059ff83..532a53c 100644
--- a/flang/lib/FIR/afforestation.cc
+++ b/flang/lib/FIR/afforestation.cc
@@ -25,9 +25,13 @@
 
 namespace Fortran::FIR {
 namespace {
-Expression *ExprRef(const parser::Expr &a) { return &a.typedExpr.get()->v; }
+Expression *ExprRef(const parser::Expr &a) {
+  CHECK(a.typedExpr);
+  CHECK(a.typedExpr->v);
+  return &*a.typedExpr->v;
+}
 Expression *ExprRef(const common::Indirection<parser::Expr> &a) {
-  return &a.value().typedExpr.get()->v;
+  return ExprRef(a.value());
 }
 
 template<typename STMTTYPE, typename CT>
diff --git a/flang/lib/evaluate/expression.h b/flang/lib/evaluate/expression.h
index 6bdc513..4257ef6 100644
--- a/flang/lib/evaluate/expression.h
+++ b/flang/lib/evaluate/expression.h
@@ -837,11 +837,12 @@ public:
 
 // This wrapper class is used, by means of a forward reference with
 // an owning pointer, to cache analyzed expressions in parse tree nodes.
+// v is nullopt if an error occurred during expression analysis.
 struct GenericExprWrapper {
-  GenericExprWrapper(Expr<SomeType> &&x) : v{std::move(x)} {}
+  GenericExprWrapper(std::optional<Expr<SomeType>> &&x) : v{std::move(x)} {}
   ~GenericExprWrapper();
   bool operator==(const GenericExprWrapper &) const;
-  Expr<SomeType> v;
+  std::optional<Expr<SomeType>> v;
 };
 
 std::ostream &DerivedTypeSpecAsFortran(
diff --git a/flang/lib/semantics/expression.cc b/flang/lib/semantics/expression.cc
index 9c727590f..7a48f3c 100644
--- a/flang/lib/semantics/expression.cc
+++ b/flang/lib/semantics/expression.cc
@@ -1859,10 +1859,7 @@ void FixMisparsedFunctionReference(
 // Common handling of parser::Expr and Parser::Variable
 template<typename PARSED>
 MaybeExpr ExpressionAnalyzer::ExprOrVariable(const PARSED &x) {
-  if (x.typedExpr) {
-    // Expression was already checked by ExprChecker
-    return std::make_optional<Expr<SomeType>>(x.typedExpr->v);
-  } else {
+  if (!x.typedExpr) {  // not yet analyzed
     FixMisparsedFunctionReference(context_, x.u);
     MaybeExpr result;
     if constexpr (std::is_same_v<PARSED, parser::Expr>) {
@@ -1873,9 +1870,8 @@ MaybeExpr ExpressionAnalyzer::ExprOrVariable(const PARSED &x) {
     } else {
       result = Analyze(x.u);
     }
-    if (result.has_value()) {
-      x.typedExpr.reset(new GenericExprWrapper{common::Clone(*result)});
-    } else if (!fatalErrors_) {
+    x.typedExpr.reset(new GenericExprWrapper{std::move(result)});
+    if (!x.typedExpr->v.has_value()) {
       if (!context_.AnyFatalError()) {
 #if DUMP_ON_FAILURE
         parser::DumpTree(std::cout << "Expression analysis failed on: ", x);
@@ -1885,8 +1881,8 @@ MaybeExpr ExpressionAnalyzer::ExprOrVariable(const PARSED &x) {
       }
       fatalErrors_ = true;
     }
-    return result;
   }
+  return x.typedExpr->v;
 }
 
 MaybeExpr ExpressionAnalyzer::Analyze(const parser::Expr &expr) {
diff --git a/flang/lib/semantics/tools.h b/flang/lib/semantics/tools.h
index aa9e775..5ad43b3 100644
--- a/flang/lib/semantics/tools.h
+++ b/flang/lib/semantics/tools.h
@@ -107,7 +107,8 @@ bool ExprTypeKindIsDefault(
 
 struct GetExprHelper {
   const SomeExpr *Get(const parser::Expr::TypedExpr &x) {
-    return x ? &x->v : nullptr;
+    CHECK(x);
+    return x->v ? &*x->v : nullptr;
   }
   const SomeExpr *Get(const parser::Expr &x) { return Get(x.typedExpr); }
   const SomeExpr *Get(const parser::Variable &x) { return Get(x.typedExpr); }
-- 
2.7.4