[flang][MSVC] Use list<Message> rather than forward_list<> in Messages

The implementation of Messages with forward_list<> makes some
nonstandard assumptions about the validity of iterators that don't
hold up with MSVC's implementation.  Use list<> instead.  The
measured performance is comparable.

This change obviated a distinction between two member functions
of Messages, and the uses of one have been replaced with calls
to the other.

Similar usage in CharBuffer was also replaced for consistency.

Differential revision: https://reviews.llvm.org/D91210

diff --git a/flang/include/flang/Parser/char-buffer.h b/flang/include/flang/Parser/char-buffer.h
index 1879e19..9d3812c 100644 (file)
--- a/flang/include/flang/Parser/char-buffer.h
+++ b/flang/include/flang/Parser/char-buffer.h
@@ -13,7 +13,7 @@
 // a stream of bytes.
 
 #include <cstddef>
-#include <forward_list>
+#include <list>
 #include <string>
 #include <utility>
 #include <vector>
@@ -24,13 +24,12 @@ class CharBuffer {
 public:
   CharBuffer() {}
   CharBuffer(CharBuffer &&that)
-      : blocks_(std::move(that.blocks_)), last_{that.last_},
-        bytes_{that.bytes_}, lastBlockEmpty_{that.lastBlockEmpty_} {
+      : blocks_(std::move(that.blocks_)), bytes_{that.bytes_},
+        lastBlockEmpty_{that.lastBlockEmpty_} {
     that.clear();
   }
   CharBuffer &operator=(CharBuffer &&that) {
     blocks_ = std::move(that.blocks_);
-    last_ = that.last_;
     bytes_ = that.bytes_;
     lastBlockEmpty_ = that.lastBlockEmpty_;
     that.clear();
@@ -42,7 +41,6 @@ public:
 
   void clear() {
     blocks_.clear();
-    last_ = blocks_.end();
     bytes_ = 0;
     lastBlockEmpty_ = false;
   }
@@ -65,8 +63,7 @@ private:
   };
 
   int LastBlockOffset() const { return bytes_ % Block::capacity; }
-  std::forward_list<Block> blocks_;
-  std::forward_list<Block>::iterator last_{blocks_.end()};
+  std::list<Block> blocks_;
   std::size_t bytes_{0};
   bool lastBlockEmpty_{false};
 };

diff --git a/flang/include/flang/Parser/instrumented-parser.h b/flang/include/flang/Parser/instrumented-parser.h
index 1bc1c52..9d958b9 100644 (file)
--- a/flang/include/flang/Parser/instrumented-parser.h
+++ b/flang/include/flang/Parser/instrumented-parser.h
@@ -63,7 +63,7 @@ public:
         Messages messages{std::move(state.messages())};
         std::optional<resultType> result{parser_.Parse(state)};
         log->Note(at, tag_, result.has_value(), state);
-        state.messages().Restore(std::move(messages));
+        state.messages().Annex(std::move(messages));
         return result;
       }
     }

diff --git a/flang/include/flang/Parser/message.h b/flang/include/flang/Parser/message.h
index cbb42df..8286384 100644 (file)
--- a/flang/include/flang/Parser/message.h
+++ b/flang/include/flang/Parser/message.h
@@ -21,6 +21,7 @@
 #include <cstddef>
 #include <cstring>
 #include <forward_list>
+#include <list>
 #include <optional>
 #include <string>
 #include <utility>
@@ -213,43 +214,22 @@ private:
 class Messages {
 public:
   Messages() {}
-  Messages(Messages &&that) : messages_{std::move(that.messages_)} {
-    if (!messages_.empty()) {
-      last_ = that.last_;
-      that.ResetLastPointer();
-    }
-  }
+  Messages(Messages &&that) : messages_{std::move(that.messages_)} {}
   Messages &operator=(Messages &&that) {
     messages_ = std::move(that.messages_);
-    if (messages_.empty()) {
-      ResetLastPointer();
-    } else {
-      last_ = that.last_;
-      that.ResetLastPointer();
-    }
     return *this;
   }
 
-  std::forward_list<Message> &messages() { return messages_; }
+  std::list<Message> &messages() { return messages_; }
   bool empty() const { return messages_.empty(); }
-  void clear();
+  void clear() { messages_.clear(); }
 
   template <typename... A> Message &Say(A &&...args) {
-    last_ = messages_.emplace_after(last_, std::forward<A>(args)...);
-    return *last_;
+    return messages_.emplace_back(std::forward<A>(args)...);
   }
 
   void Annex(Messages &&that) {
-    if (!that.messages_.empty()) {
-      messages_.splice_after(last_, that.messages_);
-      last_ = that.last_;
-      that.ResetLastPointer();
-    }
-  }
-
-  void Restore(Messages &&that) {
-    that.Annex(std::move(*this));
-    *this = std::move(that);
+    messages_.splice(messages_.end(), that.messages_);
   }
 
   bool Merge(const Message &);
@@ -262,10 +242,7 @@ public:
   bool AnyFatalError() const;
 
 private:
-  void ResetLastPointer() { last_ = messages_.before_begin(); }
-
-  std::forward_list<Message> messages_;
-  std::forward_list<Message>::iterator last_{messages_.before_begin()};
+  std::list<Message> messages_;
 };
 
 class ContextualMessages {

diff --git a/flang/lib/Parser/basic-parsers.h b/flang/lib/Parser/basic-parsers.h
index 7f349d7..03238b5 100644 (file)
--- a/flang/lib/Parser/basic-parsers.h
+++ b/flang/lib/Parser/basic-parsers.h
@@ -108,7 +108,7 @@ public:
     ParseState backtrack{state};
     std::optional<resultType> result{parser_.Parse(state)};
     if (result) {
-      state.messages().Restore(std::move(messages));
+      state.messages().Annex(std::move(messages));
     } else {
       state = std::move(backtrack);
       state.messages() = std::move(messages);
@@ -311,7 +311,7 @@ public:
         ParseRest<1>(result, state, backtrack);
       }
     }
-    state.messages().Restore(std::move(messages));
+    state.messages().Annex(std::move(messages));
     return result;
   }
 
@@ -371,7 +371,7 @@ public:
     }
     Messages messages{std::move(state.messages())};
     if (std::optional<resultType> ax{pa_.Parse(state)}) {
-      state.messages().Restore(std::move(messages));
+      state.messages().Annex(std::move(messages));
       return ax;
     }
     messages.Annex(std::move(state.messages()));

diff --git a/flang/lib/Parser/char-buffer.cpp b/flang/lib/Parser/char-buffer.cpp
index 780d7e8..0649d2c 100644 (file)
--- a/flang/lib/Parser/char-buffer.cpp
+++ b/flang/lib/Parser/char-buffer.cpp
@@ -18,14 +18,13 @@ char *CharBuffer::FreeSpace(std::size_t &n) {
   int offset{LastBlockOffset()};
   if (blocks_.empty()) {
     blocks_.emplace_front();
-    last_ = blocks_.begin();
     lastBlockEmpty_ = true;
   } else if (offset == 0 && !lastBlockEmpty_) {
-    last_ = blocks_.emplace_after(last_);
+    blocks_.emplace_back();
     lastBlockEmpty_ = true;
   }
   n = Block::capacity - offset;
-  return last_->data + offset;
+  return blocks_.back().data + offset;
 }
 
 void CharBuffer::Claim(std::size_t n) {

diff --git a/flang/lib/Parser/message.cpp b/flang/lib/Parser/message.cpp
index 6819ee4..bef55f3 100644 (file)
--- a/flang/lib/Parser/message.cpp
+++ b/flang/lib/Parser/message.cpp
@@ -260,11 +260,6 @@ bool Message::AtSameLocation(const Message &that) const {
       location_, that.location_);
 }
 
-void Messages::clear() {
-  messages_.clear();
-  ResetLastPointer();
-}
-
 bool Messages::Merge(const Message &msg) {
   if (msg.IsMergeable()) {
     for (auto &m : messages_) {
@@ -284,12 +279,12 @@ void Messages::Merge(Messages &&that) {
       if (Merge(that.messages_.front())) {
         that.messages_.pop_front();
       } else {
-        messages_.splice_after(
-            last_, that.messages_, that.messages_.before_begin());
-        ++last_;
+        auto next{that.messages_.begin()};
+        ++next;
+        messages_.splice(
+            messages_.end(), that.messages_, that.messages_.begin(), next);
       }
     }
-    that.ResetLastPointer();
   }
 }