[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

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};
 };

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;
       }
     }

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 {

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()));

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) {

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();
   }
 }