From 84ebba9e53523463f6f762b242c4fa08bf30c0a2 Mon Sep 17 00:00:00 2001 From: peter klausler Date: Tue, 17 Apr 2018 10:28:25 -0700 Subject: [PATCH] [flang] Avoid std::shared_ptr<> in favor of reference counting. Original-commit: flang-compiler/f18@116c9881c9d0c523c19a3201027b5c4974f6416e Reviewed-on: https://github.com/flang-compiler/f18/pull/59 Tree-same-pre-rewrite: false --- flang/lib/parser/basic-parsers.h | 91 ++++++++++------------------ flang/lib/parser/indirection.h | 4 -- flang/lib/parser/message.h | 37 +++++------ flang/lib/parser/parse-state.h | 60 +++++------------- flang/lib/parser/reference-counted.h | 66 ++++++++++++++++++++ 5 files changed, 130 insertions(+), 128 deletions(-) create mode 100644 flang/lib/parser/reference-counted.h diff --git a/flang/lib/parser/basic-parsers.h b/flang/lib/parser/basic-parsers.h index df7366c1eb3d..da3bf2a441ab 100644 --- a/flang/lib/parser/basic-parsers.h +++ b/flang/lib/parser/basic-parsers.h @@ -79,7 +79,7 @@ public: constexpr BacktrackingParser(const A &parser) : parser_{parser} {} std::optional Parse(ParseState *state) const { Messages messages{std::move(state->messages())}; - MessageContext context{state->context()}; + Message::Context context{state->context()}; ParseState backtrack{*state}; std::optional result{parser_.Parse(state)}; if (result.has_value()) { @@ -89,7 +89,7 @@ public: } else { state->swap(backtrack); state->messages().swap(messages); - state->set_context(context); + state->set_context(std::move(context)); } return result; } @@ -239,75 +239,46 @@ public: constexpr AlternativeParser(const PA &pa, const PB &pb) : pa_{pa}, pb_{pb} {} std::optional Parse(ParseState *state) const { Messages messages{std::move(state->messages())}; - MessageContext context{state->context()}; + Message::Context context{state->context()}; ParseState backtrack{*state}; - int depth{state->alternativeDepth()}; - bool deferred{state->deferMessages()}; - if (!deferred) { - if (depth == 0 && state->messages().empty()) { - CHECK(!state->blockedMessages()); - state->set_deferMessages(true); - } - } - state->set_alternativeDepth(depth + 1); if (std::optional ax{pa_.Parse(state)}) { - if (!deferred && state->blockedMessages()) { - // We deferred messages but now we need them. Regenerate. - state->swap(backtrack); - state->messages().swap(messages); - state->set_context(context); - return pa_.Parse(state); - } - state->set_alternativeDepth(depth) - .set_deferMessages(deferred) - .set_context(context) - .messages().Annex(messages); + // preserve any new messages + messages.Annex(state->messages()); + state->messages().swap(messages); return ax; } - ParseState backtrack2{backtrack}; +#if 0 // needed below if "tied" messages are to be saved + auto start = backtrack.GetLocation(); +#endif + ParseState paState{std::move(*state)}; state->swap(backtrack); - if (!deferred) { - if (depth == 0 && state->messages().empty()) { - CHECK(!state->blockedMessages()); - state->set_deferMessages(true); - } - } - state->set_alternativeDepth(depth + 1); + state->set_context(std::move(context)); if (std::optional bx{pb_.Parse(state)}) { - if (!deferred && state->blockedMessages()) { - state->swap(backtrack2); - state->messages().swap(messages); - state->set_context(context); - return pb_.Parse(state); - } - state->set_alternativeDepth(depth) - .set_deferMessages(deferred) - .set_context(context) - .messages().Annex(messages); + // preserve any new messages + messages.Annex(state->messages()); + state->messages().swap(messages); return bx; } // Both alternatives failed. Retain the state (and messages) from the // alternative parse that went the furthest. - if (backtrack.GetLocation() >= state->GetLocation()) { - if (!deferred && backtrack.blockedMessages()) { - state->swap(backtrack2); - state->messages().swap(messages); - state->set_context(context); - return pa_.Parse(state); - } - state->swap(backtrack); + auto paEnd = paState.GetLocation(); + auto pbEnd = state->GetLocation(); + if (paEnd > pbEnd) { + messages.Annex(paState.messages()); + state->swap(paState); + } else if (paEnd < pbEnd) { + messages.Annex(state->messages()); } else { - if (!deferred && state->blockedMessages()) { - state->swap(backtrack2); - state->messages().swap(messages); - state->set_context(context); - return pb_.Parse(state); + // It's a tie. + messages.Annex(paState.messages()); +#if 0 + if (paEnd > start) { + // Both parsers consumed text; retain messages from both. + messages.Annex(state->messages()); } +#endif } - state->set_alternativeDepth(depth) - .set_deferMessages(deferred) - .set_context(context) - .messages().Annex(messages); + state->messages().swap(messages); return {}; } @@ -342,7 +313,7 @@ public: constexpr RecoveryParser(const PA &pa, const PB &pb) : pa_{pa}, pb_{pb} {} std::optional Parse(ParseState *state) const { Messages messages{std::move(state->messages())}; - MessageContext context{state->context()}; + Message::Context context{state->context()}; ParseState backtrack{*state}; std::optional ax{pa_.Parse(state)}; messages.Annex(state->messages()); @@ -351,7 +322,7 @@ public: return ax; } state->swap(backtrack); - state->set_context(context); + state->set_context(std::move(context)); std::optional bx{pb_.Parse(state)}; state->messages().swap(messages); if (bx.has_value()) { diff --git a/flang/lib/parser/indirection.h b/flang/lib/parser/indirection.h index 548021cd14cf..1edf16e0e039 100644 --- a/flang/lib/parser/indirection.h +++ b/flang/lib/parser/indirection.h @@ -16,10 +16,6 @@ template class Indirection { public: using element_type = A; Indirection() = delete; - Indirection(A *&p) : p_{p} { - CHECK(p_ && "assigning null pointer to Indirection"); - p = nullptr; - } Indirection(A *&&p) : p_{p} { CHECK(p_ && "assigning null pointer to Indirection"); p = nullptr; diff --git a/flang/lib/parser/message.h b/flang/lib/parser/message.h index 9cfc123ac3cc..39fd08f92213 100644 --- a/flang/lib/parser/message.h +++ b/flang/lib/parser/message.h @@ -6,9 +6,9 @@ #include "idioms.h" #include "provenance.h" +#include "reference-counted.h" #include #include -#include #include #include #include @@ -80,34 +80,31 @@ private: std::size_t bytes_{1}; }; -class Message; -using MessageContext = std::shared_ptr; -class Message { +class Message : public ReferenceCounted { public: + using Context = CountedReference; + Message() {} - Message(const Message &) = default; Message(Message &&) = default; - Message &operator=(const Message &that) = default; Message &operator=(Message &&that) = default; // TODO: Change these to cover ranges of provenance - Message(Provenance p, MessageFixedText t, MessageContext c = nullptr) + Message(Provenance p, MessageFixedText t, Message *c = nullptr) : provenance_{p}, text_{t}, context_{c}, isFatal_{t.isFatal()} {} - Message(Provenance p, MessageFormattedText &&s, MessageContext c = nullptr) - : provenance_{p}, string_{s.MoveString()}, context_{c}, isFatal_{ - s.isFatal()} {} - Message(Provenance p, MessageExpectedText t, MessageContext c = nullptr) + Message(Provenance p, MessageFormattedText &&s, Message *c = nullptr) + : provenance_{p}, string_{s.MoveString()}, context_{c}, + isFatal_{s.isFatal()} {} + Message(Provenance p, MessageExpectedText t, Message *c = nullptr) : provenance_{p}, text_{t.AsMessageFixedText()}, isExpectedText_{true}, context_{c}, isFatal_{true} {} - Message(const char *csl, MessageFixedText t, MessageContext c = nullptr) - : cookedSourceLocation_{csl}, text_{t}, context_{c}, isFatal_{t.isFatal()} { - } - Message(const char *csl, MessageFormattedText &&s, MessageContext c = nullptr) + Message(const char *csl, MessageFixedText t, Message *c = nullptr) + : cookedSourceLocation_{csl}, text_{t}, context_{c}, isFatal_{t.isFatal()} {} + Message(const char *csl, MessageFormattedText &&s, Message *c = nullptr) : cookedSourceLocation_{csl}, string_{s.MoveString()}, context_{c}, isFatal_{s.isFatal()} {} - Message(const char *csl, MessageExpectedText t, MessageContext c = nullptr) + Message(const char *csl, MessageExpectedText t, Message *c = nullptr) : cookedSourceLocation_{csl}, text_{t.AsMessageFixedText()}, isExpectedText_{true}, context_{c}, isFatal_{true} {} @@ -123,20 +120,24 @@ public: Provenance provenance() const { return provenance_; } const char *cookedSourceLocation() const { return cookedSourceLocation_; } - MessageContext context() const { return context_; } + Context context() const { return context_; } bool isFatal() const { return isFatal_; } Provenance Emit( std::ostream &, const CookedSource &, bool echoSourceLine = true) const; + void TakeReference() { ++refCount_; } + void DropReference() { if (--refCount_ == 0) { delete this; } } + private: Provenance provenance_; const char *cookedSourceLocation_{nullptr}; MessageFixedText text_; bool isExpectedText_{false}; // implies "expected '%s'"_err_en_US std::string string_; - MessageContext context_; + Context context_; bool isFatal_{false}; + int refCount_{0}; }; class Messages { diff --git a/flang/lib/parser/parse-state.h b/flang/lib/parser/parse-state.h index 10216bf6e1e1..8289284e23fb 100644 --- a/flang/lib/parser/parse-state.h +++ b/flang/lib/parser/parse-state.h @@ -35,10 +35,7 @@ public: warnOnNonstandardUsage_{that.warnOnNonstandardUsage_}, warnOnDeprecatedUsage_{that.warnOnDeprecatedUsage_}, anyErrorRecovery_{that.anyErrorRecovery_}, - anyConformanceViolation_{that.anyConformanceViolation_}, - alternativeDepth_{that.alternativeDepth_}, - deferMessages_{that.deferMessages_}, blockedMessages_{ - that.blockedMessages_} {} + anyConformanceViolation_{that.anyConformanceViolation_} {} ParseState(ParseState &&that) : p_{that.p_}, limit_{that.limit_}, messages_{std::move(that.messages_)}, context_{std::move(that.context_)}, userState_{that.userState_}, @@ -47,10 +44,7 @@ public: warnOnNonstandardUsage_{that.warnOnNonstandardUsage_}, warnOnDeprecatedUsage_{that.warnOnDeprecatedUsage_}, anyErrorRecovery_{that.anyErrorRecovery_}, - anyConformanceViolation_{that.anyConformanceViolation_}, - alternativeDepth_{that.alternativeDepth_}, - deferMessages_{that.deferMessages_}, blockedMessages_{ - that.blockedMessages_} {} + anyConformanceViolation_{that.anyConformanceViolation_} {} ParseState &operator=(ParseState &&that) { swap(that); return *this; @@ -75,11 +69,15 @@ public: UserState *userState() const { return userState_; } void set_userState(UserState *u) { userState_ = u; } - MessageContext context() const { return context_; } - ParseState &set_context(MessageContext c) { + Message::Context context() const { return context_; } + ParseState &set_context(const Message::Context &c) { context_ = c; return *this; } + ParseState &set_context(Message::Context &&c) { + context_ = std::move(c); + return *this; + } bool inFixedForm() const { return inFixedForm_; } ParseState &set_inFixedForm(bool yes) { @@ -111,25 +109,10 @@ public: return *this; } - int alternativeDepth() const { return alternativeDepth_; } - ParseState &set_alternativeDepth(int d) { - alternativeDepth_ = d; - return *this; - } - - bool deferMessages() const { return deferMessages_; } - ParseState &set_deferMessages(bool yes) { - deferMessages_ = yes; - return *this; - } - - bool blockedMessages() const { return blockedMessages_; } - const char *GetLocation() const { return p_; } - MessageContext &PushContext(MessageFixedText text) { - context_ = std::make_shared(p_, text, context_); - return context_; + void PushContext(MessageFixedText text) { + context_ = Message::Context{new Message{p_, text, context_.get()}}; } void PopContext() { @@ -143,25 +126,13 @@ public: void Say(MessageExpectedText &&t) { return Say(p_, std::move(t)); } void Say(const char *at, MessageFixedText t) { - if (deferMessages_) { - blockedMessages_ = true; - } else { - messages_.Put(Message{at, t, context_}); - } + messages_.Put(Message{at, t, context_.get()}); } void Say(const char *at, MessageFormattedText &&t) { - if (deferMessages_) { - blockedMessages_ = true; - } else { - messages_.Put(Message{at, std::move(t), context_}); - } + messages_.Put(Message{at, std::move(t), context_.get()}); } void Say(const char *at, MessageExpectedText &&t) { - if (deferMessages_) { - blockedMessages_ = true; - } else { - messages_.Put(Message{at, std::move(t), context_}); - } + messages_.Put(Message{at, std::move(t), context_.get()}); } bool IsAtEnd() const { return p_ >= limit_; } @@ -197,7 +168,7 @@ private: // Accumulated messages and current nested context. Messages messages_; - MessageContext context_; + Message::Context context_; UserState *userState_{nullptr}; @@ -208,9 +179,6 @@ private: bool warnOnDeprecatedUsage_{false}; bool anyErrorRecovery_{false}; bool anyConformanceViolation_{false}; - int alternativeDepth_{0}; - bool deferMessages_{false}; - bool blockedMessages_{false}; // NOTE: Any additions or modifications to these data members must also be // reflected in the copy and move constructors defined at the top of this // class definition! diff --git a/flang/lib/parser/reference-counted.h b/flang/lib/parser/reference-counted.h new file mode 100644 index 000000000000..ab9131a1e2e7 --- /dev/null +++ b/flang/lib/parser/reference-counted.h @@ -0,0 +1,66 @@ +#ifndef FORTRAN_PARSER_REFERENCE_COUNTED_H_ +#define FORTRAN_PARSER_REFERENCE_COUNTED_H_ + +// A template class of smart pointers to objects with their own +// reference counting object lifetimes that's lighter weight +// than std::shared_ptr<>. Not thread-safe. + +namespace Fortran { +namespace parser { + +template class ReferenceCounted { +public: + ReferenceCounted() {} + void TakeReference() { ++references_; } + void DropReference() { + if (--references_ == 0) { + delete static_cast(this); + } + } +private: + int references_{0}; +}; + +template class CountedReference { +public: + using type = A; + CountedReference() {} + CountedReference(type *m) : p_{m} { Take(); } + CountedReference(const CountedReference &c) : p_{c.p_} { Take(); } + CountedReference(CountedReference &&c) : p_{c.p_} { c.p_ = nullptr; } + CountedReference &operator=(const CountedReference &c) { + Drop(); + p_ = c.p_; + Take(); + return *this; + } + CountedReference &operator=(CountedReference &&c) { + Drop(); + p_ = c.p_; + c.p_ = nullptr; + return *this; + } + ~CountedReference() { Drop(); } + operator bool() const { return p_ != nullptr; } + type *get() const { return p_; } + type &operator*() const { return *p_; } + type *operator->() const { return p_; } + +private: + void Take() { + if (p_) { + p_->TakeReference(); + } + } + void Drop() { + if (p_) { + p_->DropReference(); + p_ = nullptr; + } + } + + type *p_{nullptr}; +}; +} // namespace parser +} // namespace Fortran +#endif // FORTRAN_PARSER_REFERENCE_COUNTED_H_