[flang-commits] [flang] cc575dd - [flang][MSVC] Use list<Message> rather than forward_list<> in Messages
peter klausler via flang-commits
flang-commits at lists.llvm.org
Wed Nov 11 16:38:51 PST 2020
Author: peter klausler
Date: 2020-11-11T16:38:38-08:00
New Revision: cc575dd2cefce3170655a026dbf058a42e1a4330
URL: https://github.com/llvm/llvm-project/commit/cc575dd2cefce3170655a026dbf058a42e1a4330
DIFF: https://github.com/llvm/llvm-project/commit/cc575dd2cefce3170655a026dbf058a42e1a4330.diff
LOG: [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
Added:
Modified:
flang/include/flang/Parser/char-buffer.h
flang/include/flang/Parser/instrumented-parser.h
flang/include/flang/Parser/message.h
flang/lib/Parser/basic-parsers.h
flang/lib/Parser/char-buffer.cpp
flang/lib/Parser/message.cpp
Removed:
################################################################################
diff --git a/flang/include/flang/Parser/char-buffer.h b/flang/include/flang/Parser/char-buffer.h
index 1879e1960c381..9d3812ce108b8 100644
--- 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 @@ class CharBuffer {
void clear() {
blocks_.clear();
- last_ = blocks_.end();
bytes_ = 0;
lastBlockEmpty_ = false;
}
@@ -65,8 +63,7 @@ class CharBuffer {
};
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 1bc1c526dc9f7..9d958b97c9b0a 100644
--- a/flang/include/flang/Parser/instrumented-parser.h
+++ b/flang/include/flang/Parser/instrumented-parser.h
@@ -63,7 +63,7 @@ template <typename PA> class InstrumentedParser {
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 cbb42df97351d..8286384bb2108 100644
--- 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 @@ class Message : public common::ReferenceCounted<Message> {
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 @@ class Messages {
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 7f349d7b4ff41..03238b5b4b3fb 100644
--- a/flang/lib/Parser/basic-parsers.h
+++ b/flang/lib/Parser/basic-parsers.h
@@ -108,7 +108,7 @@ template <typename A> class BacktrackingParser {
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 @@ template <typename PA, typename... Ps> class AlternativesParser {
ParseRest<1>(result, state, backtrack);
}
}
- state.messages().Restore(std::move(messages));
+ state.messages().Annex(std::move(messages));
return result;
}
@@ -371,7 +371,7 @@ template <typename PA, typename PB> class RecoveryParser {
}
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 780d7e89538f7..0649d2c3dde27 100644
--- 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 6819ee4d83b2f..bef55f3d8176d 100644
--- 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();
}
}
More information about the flang-commits
mailing list