From 53ae00a9c427d143cc7a6fc697cf5d9aff23b8cd Mon Sep 17 00:00:00 2001 From: peter klausler Date: Wed, 6 Mar 2019 11:52:25 -0800 Subject: [PATCH] [flang] Address review comments Original-commit: flang-compiler/f18@3ed6199fedc2ba0d527cc7dd46c5ab67a7f0f1c7 Reviewed-on: https://github.com/flang-compiler/f18/pull/316 Tree-same-pre-rewrite: false --- flang/lib/evaluate/fold.cc | 5 +-- flang/lib/evaluate/traversal.h | 60 ++++++++++++++++++++++++------ flang/lib/semantics/check-do-concurrent.cc | 31 ++++++++++----- 3 files changed, 71 insertions(+), 25 deletions(-) diff --git a/flang/lib/evaluate/fold.cc b/flang/lib/evaluate/fold.cc index f93274a..ee778e8 100644 --- a/flang/lib/evaluate/fold.cc +++ b/flang/lib/evaluate/fold.cc @@ -802,7 +802,7 @@ FOR_EACH_TYPE_AND_KIND(template class ExpressionBase) class IsConstantExprVisitor : public virtual TraversalBase { public: - explicit IsConstantExprVisitor(std::nullptr_t) {} + explicit IsConstantExprVisitor(int) { result() = true; } template void Handle(const TypeParamInquiry &inq) { Check(inq.parameter().template get().attr() == @@ -833,8 +833,7 @@ private: }; bool IsConstantExpr(const Expr &expr) { - Traversal traverser{nullptr}; - return !traverser.Traverse(expr).has_value(); // only no news is good news + return Traversal{0}.Traverse(expr); } std::optional ToInt64(const Expr &expr) { diff --git a/flang/lib/evaluate/traversal.h b/flang/lib/evaluate/traversal.h index 9504391..2f6148e 100644 --- a/flang/lib/evaluate/traversal.h +++ b/flang/lib/evaluate/traversal.h @@ -19,22 +19,54 @@ #include // Implements an expression traversal utility framework. +// See fold.cc to see how this framework is used to implement detection +// of constant expressions. +// +// To use, define one or more client visitation classes of the form: +// class MyVisitor : public virtual TraversalBase { +// explicit MyVisitor(ARGTYPE); // single-argument constructor +// void Handle(const T1 &); // callback for type T1 objects +// void Pre(const T2 &); // callback before visiting T2 +// void Post(const T2 &); // callback after visiting T2 +// ... +// }; +// RESULT should have some default-constructible type. +// Then instantiate and construct a Traversal and its embedded MyVisitor via: +// Traversal t{value}; // value is ARGTYPE && +// and call: +// RESULT result{t.Traverse(topLevelExpr)}; +// Within the callback routines (Handle, Pre, Post), one may call +// void Stop(); // to end traversal +// void Return(...); // to emplace-construct the result and end traversal +// RESULT &result(); // to reference the result to define or update it +// For any given expression object type T for which a callback is defined +// in any visitor class, the callback must be distinct from all others. +// Further, if there is a Handle(const T &) callback, there cannot be a +// Pre() or a Post(). + namespace Fortran::evaluate { template class TraversalBase { public: using Result = RESULT; - // Note the weird return type; it distinguishes this default Handle - // from any void-valued override. + + Result &result() { return result_; } + + // Note the odd return type; it distinguishes these default callbacks + // from any void-valued client callback. template std::nullptr_t Handle(const A &) { return nullptr; } - template void Pre(const A &) {} - template void Post(const A &) {} - template void Return(A &&... x) { - result_.emplace(std::move(x)...); + template std::nullptr_t Pre(const A &) { return nullptr; } + template std::nullptr_t Post(const A &) { return nullptr; } + + void Stop() { done_ = true; } + void Return(RESULT &&x) { + result_ = std::move(x); + Stop(); } protected: - std::optional result_; + bool done_{false}; + Result result_; }; // Descend() is a helper function template for Traversal::Visit(). @@ -182,29 +214,33 @@ public: using A::Handle..., A::Pre..., A::Post...; private: - using TraversalBase::result_; + using TraversalBase::done_, TraversalBase::result_; public: template Traversal(B... x) : A{x}... {} - template std::optional Traverse(const B &x) { + template Result Traverse(const B &x) { Visit(x); return std::move(result_); } // TODO: make private, make Descend instances friends template void Visit(const B &x) { - if (!result_.has_value()) { + if (!done_) { if constexpr (std::is_same_v, std::nullptr_t>) { // No visitation class defines Handle(B), so try Pre()/Post(). Pre(x); - if (!result_.has_value()) { + if (!done_) { descend::Descend(*this, x); - if (!result_.has_value()) { + if (!done_) { Post(x); } } } else { + static_assert( + std::is_same_v, std::nullptr_t>); + static_assert( + std::is_same_v, std::nullptr_t>); Handle(x); } } diff --git a/flang/lib/semantics/check-do-concurrent.cc b/flang/lib/semantics/check-do-concurrent.cc index 6e06ba29..1f3698e 100644 --- a/flang/lib/semantics/check-do-concurrent.cc +++ b/flang/lib/semantics/check-do-concurrent.cc @@ -14,10 +14,12 @@ #include "check-do-concurrent.h" #include "attr.h" +#include "expression.h" #include "scope.h" #include "semantics.h" #include "symbol.h" #include "type.h" +#include "../evaluate/traversal.h" #include "../parser/message.h" #include "../parser/parse-tree-visitor.h" @@ -218,13 +220,6 @@ private: using CS = std::vector; -struct GatherSymbols { - CS symbols; - template constexpr bool Pre(const T &) { return true; } - template constexpr void Post(const T &) {} - void Post(const parser::Name &name) { symbols.push_back(name.symbol); } -}; - static bool IntegerVariable(const Symbol &variable) { return variable.GetType()->IsNumeric(common::TypeCategory::Integer); } @@ -300,10 +295,26 @@ static CS GatherLocalVariableNames( } return names; } + static CS GatherReferencesFromExpression(const parser::Expr &expression) { - GatherSymbols gatherSymbols; - parser::Walk(expression, gatherSymbols); - return gatherSymbols.symbols; + // Use the new expression traversal framework if possible, for testing. + if (expression.typedExpr.has_value()) { + struct CollectSymbols : public virtual evaluate::TraversalBase { + explicit CollectSymbols(int) {} + void Handle(const Symbol *symbol) { result().push_back(symbol); } + }; + return evaluate::Traversal{0}.Traverse( + expression.typedExpr.value()); + } else { + struct GatherSymbols { + CS symbols; + template constexpr bool Pre(const T &) { return true; } + template constexpr void Post(const T &) {} + void Post(const parser::Name &name) { symbols.push_back(name.symbol); } + } gatherSymbols; + parser::Walk(expression, gatherSymbols); + return gatherSymbols.symbols; + } } // Find a canonical DO CONCURRENT and enforce semantics checks on its body -- 2.7.4