[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

diff --git a/flang/lib/parser/basic-parsers.h b/flang/lib/parser/basic-parsers.h
index f952311..4706048 100644 (file)
--- 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<Success> 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<Success> 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<resultType> 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<resultType> 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<resultType> 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<resultType> 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 (file)
--- 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 (file)
--- 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 (file)
--- 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());