From 984db1b650bf7e81d903efb38b3d7d4347d55d38 Mon Sep 17 00:00:00 2001 From: peter klausler Date: Tue, 17 Apr 2018 14:45:12 -0700 Subject: [PATCH] [flang] Remove questionable swap() routines in ParseState and Messages. Original-commit: flang-compiler/f18@9a8155ebca1b3f0a7d345a7a832dcc765623f096 Reviewed-on: https://github.com/flang-compiler/f18/pull/59 Tree-same-pre-rewrite: false --- flang/lib/parser/basic-parsers.h | 50 +++++++++++++++++++++---------- flang/lib/parser/message.h | 28 +++++++++-------- flang/lib/parser/parse-state.h | 65 +++++++++++++++++++++++++++++++--------- flang/lib/parser/parsing.cc | 2 -- 4 files changed, 101 insertions(+), 44 deletions(-) diff --git a/flang/lib/parser/basic-parsers.h b/flang/lib/parser/basic-parsers.h index f952311..4706048 100644 --- a/flang/lib/parser/basic-parsers.h +++ b/flang/lib/parser/basic-parsers.h @@ -85,10 +85,10 @@ public: if (result.has_value()) { // preserve any new messages messages.Annex(state->messages()); - state->messages().swap(messages); + state->messages() = std::move(messages); } else { - state->swap(backtrack); - state->messages().swap(messages); + *state = std::move(backtrack); + state->messages() = std::move(messages); state->set_context(std::move(context)); } return result; @@ -112,7 +112,7 @@ public: std::optional Parse(ParseState *state) const { Messages messages{std::move(state->messages())}; ParseState forked{*state}; - state->messages().swap(messages); + state->messages() = std::move(messages); if (parser_.Parse(&forked)) { return {}; } @@ -137,7 +137,7 @@ public: std::optional Parse(ParseState *state) const { Messages messages{std::move(state->messages())}; ParseState forked{*state}; - state->messages().swap(messages); + state->messages() = std::move(messages); if (parser_.Parse(&forked).has_value()) { return {Success{}}; } @@ -244,16 +244,16 @@ public: if (std::optional ax{pa_.Parse(state)}) { // preserve any new messages messages.Annex(state->messages()); - state->messages().swap(messages); + state->messages() = std::move(messages); return ax; } ParseState paState{std::move(*state)}; - state->swap(backtrack); + *state = std::move(backtrack); state->set_context(std::move(context)); if (std::optional bx{pb_.Parse(state)}) { // preserve any new messages messages.Annex(state->messages()); - state->messages().swap(messages); + state->messages() = std::move(messages); return bx; } // Both alternatives failed. Retain the state (and messages) from the @@ -262,14 +262,14 @@ public: auto pbEnd = state->GetLocation(); if (paEnd > pbEnd) { messages.Annex(paState.messages()); - state->swap(paState); + *state = std::move(paState); } else if (paEnd < pbEnd) { messages.Annex(state->messages()); } else { // It's a tie. messages.Annex(paState.messages()); } - state->messages().swap(messages); + state->messages() = std::move(messages); return {}; } @@ -306,17 +306,37 @@ public: Messages messages{std::move(state->messages())}; Message::Context context{state->context()}; ParseState backtrack{*state}; + bool originallyDeferred{state->deferMessages()}; +#if 0 + state->set_deferMessages(true); +#endif std::optional ax{pa_.Parse(state)}; - messages.Annex(state->messages()); +#if 0 + if (!originallyDeferred && state->anyDeferredMessages()) { + CHECK(state->messages().empty()); + *state = backtrack; + state->set_context(context); + ax = pa_.Parse(state); + CHECK(!state->messages().empty()); + } +#endif if (ax.has_value()) { - state->messages().swap(messages); + messages.Annex(state->messages()); + state->messages() = std::move(messages); + state->set_context(std::move(context)); + state->set_deferMessages(originallyDeferred); return ax; } - state->swap(backtrack); - state->set_context(std::move(context)); + Messages paMessages{std::move(state->messages())}; + *state = std::move(backtrack); + state->set_deferMessages(true); std::optional bx{pb_.Parse(state)}; - state->messages().swap(messages); + CHECK(state->messages().empty()); + state->messages() = std::move(messages); + state->set_context(std::move(context)); + state->set_deferMessages(originallyDeferred); if (bx.has_value()) { + state->messages().Annex(paMessages); state->set_anyErrorRecovery(); } return bx; diff --git a/flang/lib/parser/message.h b/flang/lib/parser/message.h index d2bd9fe..3c35bd1 100644 --- a/flang/lib/parser/message.h +++ b/flang/lib/parser/message.h @@ -127,9 +127,6 @@ public: 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}; @@ -138,7 +135,6 @@ private: std::string string_; Context context_; bool isFatal_{false}; - int refCount_{0}; }; class Messages { @@ -150,18 +146,23 @@ public: explicit Messages(const CookedSource &cooked) : cooked_{cooked} {} Messages(Messages &&that) - : cooked_{that.cooked_}, messages_{std::move(that.messages_)}, - last_{that.last_} {} + : cooked_{that.cooked_}, messages_{std::move(that.messages_)} { + if (!messages_.empty()) { + last_ = that.last_; + that.last_ = that.messages_.before_begin(); + } + } Messages &operator=(Messages &&that) { - swap(that); + messages_ = std::move(that.messages_); + if (messages_.empty()) { + last_ = messages_.before_begin(); + } else { + last_ = that.last_; + that.last_ = that.messages_.before_begin(); + } return *this; } - void swap(Messages &that) { - messages_.swap(that.messages_); - std::swap(last_, that.last_); - } - bool empty() const { return messages_.empty(); } iterator begin() { return messages_.begin(); } iterator end() { return messages_.end(); } @@ -181,7 +182,7 @@ public: } void Put(Message &&m) { -// CHECK(IsValidLocation(m)); + CHECK(IsValidLocation(m)); last_ = messages_.emplace_after(last_, std::move(m)); } @@ -193,6 +194,7 @@ public: if (!that.messages_.empty()) { messages_.splice_after(last_, that.messages_); last_ = that.last_; + that.last_ = that.messages_.before_begin(); } } diff --git a/flang/lib/parser/parse-state.h b/flang/lib/parser/parse-state.h index bdf491d..69716ad 100644 --- a/flang/lib/parser/parse-state.h +++ b/flang/lib/parser/parse-state.h @@ -35,7 +35,8 @@ public: warnOnNonstandardUsage_{that.warnOnNonstandardUsage_}, warnOnDeprecatedUsage_{that.warnOnDeprecatedUsage_}, anyErrorRecovery_{that.anyErrorRecovery_}, - anyConformanceViolation_{that.anyConformanceViolation_} {} + anyConformanceViolation_{that.anyConformanceViolation_}, + deferMessages_{that.deferMessages_} {} ParseState(ParseState &&that) : p_{that.p_}, limit_{that.limit_}, messages_{std::move(that.messages_)}, context_{std::move(that.context_)}, userState_{that.userState_}, @@ -44,18 +45,32 @@ public: warnOnNonstandardUsage_{that.warnOnNonstandardUsage_}, warnOnDeprecatedUsage_{that.warnOnDeprecatedUsage_}, anyErrorRecovery_{that.anyErrorRecovery_}, - anyConformanceViolation_{that.anyConformanceViolation_} {} - ParseState &operator=(ParseState &&that) { - swap(that); + anyConformanceViolation_{that.anyConformanceViolation_}, + deferMessages_{that.deferMessages_}, + anyDeferredMessages_{that.anyDeferredMessages_} {} + ParseState &operator=(const ParseState &that) { + p_ = that.p_, limit_ = that.limit_; + userState_ = that.userState_, inFixedForm_ = that.inFixedForm_; + encoding_ = that.encoding_, strictConformance_ = that.strictConformance_; + warnOnNonstandardUsage_ = that.warnOnNonstandardUsage_; + warnOnDeprecatedUsage_ = that.warnOnDeprecatedUsage_; + anyErrorRecovery_ = that.anyErrorRecovery_; + anyConformanceViolation_ = that.anyConformanceViolation_; + deferMessages_ = that.deferMessages_; + anyDeferredMessages_ = that.anyDeferredMessages_; return *this; } - - void swap(ParseState &that) { - constexpr std::size_t bytes{sizeof *this}; - char buffer[bytes]; - std::memcpy(buffer, this, bytes); - std::memcpy(this, &that, bytes); - std::memcpy(&that, buffer, bytes); + ParseState &operator=(ParseState &&that) { + p_ = that.p_, limit_ = that.limit_, messages_ = std::move(that.messages_); + userState_ = that.userState_, inFixedForm_ = that.inFixedForm_; + encoding_ = that.encoding_, strictConformance_ = that.strictConformance_; + warnOnNonstandardUsage_ = that.warnOnNonstandardUsage_; + warnOnDeprecatedUsage_ = that.warnOnDeprecatedUsage_; + anyErrorRecovery_ = that.anyErrorRecovery_; + anyConformanceViolation_ = that.anyConformanceViolation_; + deferMessages_ = that.deferMessages_; + anyDeferredMessages_ = that.anyDeferredMessages_; + return *this; } Messages &messages() { return messages_; } @@ -109,6 +124,14 @@ public: return *this; } + bool deferMessages() const { return deferMessages_; } + ParseState &set_deferMessages(bool yes) { + deferMessages_ = yes; + return *this; + } + + bool anyDeferredMessages() const { return anyDeferredMessages_; } + const char *GetLocation() const { return p_; } void PushContext(MessageFixedText text) { @@ -126,13 +149,25 @@ public: void Say(MessageExpectedText &&t) { return Say(p_, std::move(t)); } void Say(const char *at, MessageFixedText t) { - messages_.Say(at, t, context_.get()); + if (deferMessages_) { + anyDeferredMessages_ = true; + } else { + messages_.Say(at, t, context_.get()); + } } void Say(const char *at, MessageFormattedText &&t) { - messages_.Say(at, std::move(t), context_.get()); + if (deferMessages_) { + anyDeferredMessages_ = true; + } else { + messages_.Say(at, std::move(t), context_.get()); + } } void Say(const char *at, MessageExpectedText &&t) { - messages_.Say(at, std::move(t), context_.get()); + if (deferMessages_) { + anyDeferredMessages_ = true; + } else { + messages_.Say(at, std::move(t), context_.get()); + } } bool IsAtEnd() const { return p_ >= limit_; } @@ -179,6 +214,8 @@ private: bool warnOnDeprecatedUsage_{false}; bool anyErrorRecovery_{false}; bool anyConformanceViolation_{false}; + bool deferMessages_{false}; + bool anyDeferredMessages_{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/parsing.cc b/flang/lib/parser/parsing.cc index f06fa41..f97c4cd 100644 --- a/flang/lib/parser/parsing.cc +++ b/flang/lib/parser/parsing.cc @@ -82,10 +82,8 @@ void Parsing::Parse() { .set_warnOnDeprecatedUsage(options_.isStrictlyStandard) .set_userState(&userState); parseTree_ = program.Parse(&parseState); -#if 0 // pmk CHECK( !parseState.anyErrorRecovery() || parseState.messages().AnyFatalError()); -#endif consumedWholeFile_ = parseState.IsAtEnd(); finalRestingPlace_ = parseState.GetLocation(); messages_.Annex(parseState.messages()); -- 2.7.4