From fc89541e96062aa9376b169e976dbff65e642365 Mon Sep 17 00:00:00 2001 From: Tim Keith Date: Fri, 30 Mar 2018 19:49:00 -0700 Subject: [PATCH] [flang] Respond to review comments. Original-commit: flang-compiler/f18@d2497a6485e1dc8ea5fc7ba1dfb01d7603f57bfa Reviewed-on: https://github.com/flang-compiler/f18/pull/36 Tree-same-pre-rewrite: false --- flang/lib/semantics/resolve-names.cc | 34 +++++++++++++++++----------------- flang/lib/semantics/type.cc | 27 +++++++++++++++------------ flang/lib/semantics/type.h | 28 ++++++++++++++++++---------- 3 files changed, 50 insertions(+), 39 deletions(-) diff --git a/flang/lib/semantics/resolve-names.cc b/flang/lib/semantics/resolve-names.cc index 6a02421..4f1a21ef 100644 --- a/flang/lib/semantics/resolve-names.cc +++ b/flang/lib/semantics/resolve-names.cc @@ -17,9 +17,9 @@ class AttrsVisitor { public: void beginAttrs(); Attrs endAttrs(); - void Post(const parser::LanguageBindingSpec &x); - bool Pre(const parser::AccessSpec &x); - bool Pre(const parser::IntentSpec &x); + void Post(const parser::LanguageBindingSpec &); + bool Pre(const parser::AccessSpec &); + bool Pre(const parser::IntentSpec &); // Simple case: encountering CLASSNAME causes ATTRNAME to be set. #define HANDLE_ATTR_CLASS(CLASSNAME, ATTRNAME) \ @@ -53,7 +53,7 @@ public: #undef HANDLE_ATTR_CLASS protected: - std::unique_ptr attrs_; + std::optional attrs_; std::string langBindingName_{""}; }; @@ -141,10 +141,11 @@ private: // AttrsVisitor implementation void AttrsVisitor::beginAttrs() { CHECK(!attrs_); - attrs_ = std::make_unique(); + attrs_ = std::make_optional(); } Attrs AttrsVisitor::endAttrs() { - const auto result = attrs_ ? *attrs_ : Attrs::EMPTY; + CHECK(attrs_); + Attrs result{*attrs_}; attrs_.reset(); return result; } @@ -222,13 +223,13 @@ bool DeclTypeSpecVisitor::Pre(const parser::TypeParamValue &x) { return false; } -void DeclTypeSpecVisitor::Post(const parser::DeclarationTypeSpec::Type &x) { +void DeclTypeSpecVisitor::Post(const parser::DeclarationTypeSpec::Type &) { SetDeclTypeSpec( - DeclTypeSpec::MakeTypeDerivedType(*derivedTypeSpec_.release())); + DeclTypeSpec::MakeTypeDerivedType(std::move(derivedTypeSpec_))); } -void DeclTypeSpecVisitor::Post(const parser::DeclarationTypeSpec::Class &x) { +void DeclTypeSpecVisitor::Post(const parser::DeclarationTypeSpec::Class &) { SetDeclTypeSpec( - DeclTypeSpec::MakeClassDerivedType(*derivedTypeSpec_.release())); + DeclTypeSpec::MakeClassDerivedType(std::move(derivedTypeSpec_))); } bool DeclTypeSpecVisitor::Pre(const parser::DeclarationTypeSpec::Record &x) { // TODO @@ -281,24 +282,23 @@ KindParamValue DeclTypeSpecVisitor::GetKindParamValue( void ResolveNamesVisitor::Post(const parser::EntityDecl &x) { // TODO: may be under StructureStmt - const auto &name = std::get(x.t); + const auto &name{std::get(x.t)}; // TODO: optional ArraySpec, CoarraySpec, CharLength, Initialization - Symbol &symbol = CurrScope().GetOrMakeSymbol(name.ToString()); - symbol.attrs().Add(*attrs_); //TODO: check attribute consistency + Symbol &symbol{CurrScope().GetOrMakeSymbol(name.ToString())}; + symbol.attrs().Add(*attrs_); // TODO: check attribute consistency if (symbol.has()) { symbol.set_details(EntityDetails()); } if (EntityDetails *details = symbol.detailsIf()) { if (details->type().has_value()) { std::cerr << "ERROR: symbol already has a type declared: " - << name.ToString() << "\n"; + << name.ToString() << "\n"; } else { details->set_type(*declTypeSpec_); } } else { - std::cerr - << "ERROR: symbol already declared, can't appear in entity-decl: " - << name.ToString() << "\n"; + std::cerr << "ERROR: symbol already declared, can't appear in entity-decl: " + << name.ToString() << "\n"; } } diff --git a/flang/lib/semantics/type.cc b/flang/lib/semantics/type.cc index 790a8a3..7c3adf4 100644 --- a/flang/lib/semantics/type.cc +++ b/flang/lib/semantics/type.cc @@ -203,25 +203,28 @@ DataComponentDef::DataComponentDef(const DeclTypeSpec &type, const Name &name, } } -// All instances of IntrinsicTypeSpec live in caches and are never deleted, -// so the pointer to intrinsicTypeSpec will always be valid -// derivedTypeSpec_ is dynamically allocated and owned by the DeclTypeSpec -DeclTypeSpec::DeclTypeSpec(Category category, const DerivedTypeSpec &derivedTypeSpec) - : category_{category}, intrinsicTypeSpec_{nullptr}, - derivedTypeSpec_{new DerivedTypeSpec(derivedTypeSpec)} { - CHECK(category == TypeDerived || category == ClassDerived); -} DeclTypeSpec::DeclTypeSpec(const DeclTypeSpec &that) : category_{that.category_}, intrinsicTypeSpec_{that.intrinsicTypeSpec_} { if (category_ == TypeDerived || category_ == ClassDerived) { - derivedTypeSpec_ = new DerivedTypeSpec(*that.derivedTypeSpec_); + derivedTypeSpec_ = + std::make_unique(*that.derivedTypeSpec_); } } -DeclTypeSpec::~DeclTypeSpec() { + +DeclTypeSpec &DeclTypeSpec::operator=(const DeclTypeSpec &that) { + category_ = that.category_; + intrinsicTypeSpec_ = that.intrinsicTypeSpec_; if (category_ == TypeDerived || category_ == ClassDerived) { - delete derivedTypeSpec_; - derivedTypeSpec_ = nullptr; + derivedTypeSpec_ = + std::make_unique(*that.derivedTypeSpec_); } + return *this; +} + +DeclTypeSpec::DeclTypeSpec(Category category, std::unique_ptr &&typeSpec) + : category_{category}, intrinsicTypeSpec_{nullptr}, + derivedTypeSpec_{std::move(typeSpec)} { + CHECK(category == TypeDerived || category == ClassDerived); } std::ostream &operator<<(std::ostream &o, const DeclTypeSpec &x) { diff --git a/flang/lib/semantics/type.h b/flang/lib/semantics/type.h index f3d717d..b907815 100644 --- a/flang/lib/semantics/type.h +++ b/flang/lib/semantics/type.h @@ -48,7 +48,9 @@ public: } IntExpr() {} IntExpr(const parser::ScalarIntExpr &) { /*TODO*/ } - virtual std::ostream &Output(std::ostream &o) const { return o << "IntExpr"; } + virtual std::ostream &Output(std::ostream & o) const { + return o << "IntExpr"; + } }; // TODO @@ -116,20 +118,23 @@ public: return DeclTypeSpec{typeSpec}; } // TYPE(derived-type-spec) - static DeclTypeSpec MakeTypeDerivedType(const DerivedTypeSpec &typeSpec) { - return DeclTypeSpec{TypeDerived, typeSpec}; + static DeclTypeSpec MakeTypeDerivedType( + std::unique_ptr && typeSpec) { + return DeclTypeSpec{TypeDerived, std::move(typeSpec)}; } // CLASS(derived-type-spec) - static DeclTypeSpec MakeClassDerivedType(const DerivedTypeSpec &typeSpec) { - return DeclTypeSpec{ClassDerived, typeSpec}; + static DeclTypeSpec MakeClassDerivedType( + std::unique_ptr && typeSpec) { + return DeclTypeSpec{ClassDerived, std::move(typeSpec)}; } // TYPE(*) static DeclTypeSpec MakeTypeStar() { return DeclTypeSpec{TypeStar}; } // CLASS(*) static DeclTypeSpec MakeClassStar() { return DeclTypeSpec{ClassStar}; } - DeclTypeSpec(const DeclTypeSpec &that); - ~DeclTypeSpec(); + DeclTypeSpec(const DeclTypeSpec &); + DeclTypeSpec &operator=(const DeclTypeSpec &); + enum Category { Intrinsic, TypeDerived, ClassDerived, TypeStar, ClassStar }; Category category() const { return category_; } const IntrinsicTypeSpec &intrinsicTypeSpec() const { @@ -141,13 +146,16 @@ private: DeclTypeSpec(Category category) : category_{category} { CHECK(category == TypeStar || category == ClassStar); } - DeclTypeSpec(Category category, const DerivedTypeSpec &typeSpec); + DeclTypeSpec(Category category, std::unique_ptr &&typeSpec); DeclTypeSpec(const IntrinsicTypeSpec &intrinsicTypeSpec) - : category_{Intrinsic}, intrinsicTypeSpec_{&intrinsicTypeSpec} {} + : category_{Intrinsic}, intrinsicTypeSpec_{&intrinsicTypeSpec} { + // All instances of IntrinsicTypeSpec live in caches and are never deleted, + // so the pointer to intrinsicTypeSpec will always be valid. + } Category category_; const IntrinsicTypeSpec *intrinsicTypeSpec_{nullptr}; - const DerivedTypeSpec *derivedTypeSpec_{nullptr}; + std::unique_ptr derivedTypeSpec_; friend std::ostream &operator<<(std::ostream &, const DeclTypeSpec &); }; -- 2.7.4