From 0353262cc273fc8fe1f8e0767812d9ff84fce5ba Mon Sep 17 00:00:00 2001 From: jeremy Date: Fri, 9 Dec 2022 12:10:48 +0100 Subject: [PATCH 1/2] Correct impl of begin iterators --- src/query/v2/multiframe.cpp | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/src/query/v2/multiframe.cpp b/src/query/v2/multiframe.cpp index 4829addb2..27f9607d5 100644 --- a/src/query/v2/multiframe.cpp +++ b/src/query/v2/multiframe.cpp @@ -84,12 +84,24 @@ ValidFramesReader::ValidFramesReader(MultiFrame &multiframe) : multiframe_(multi after_last_valid_frame_ = multiframe_.frames_.data() + std::distance(multiframe.frames_.begin(), it); } -ValidFramesReader::Iterator ValidFramesReader::begin() { return Iterator{&multiframe_.frames_[0]}; } +ValidFramesReader::Iterator ValidFramesReader::begin() { + if (multiframe_.frames_[0].IsValid()) { + return Iterator{&multiframe_.frames_[0]}; + } + return end(); +} + ValidFramesReader::Iterator ValidFramesReader::end() { return Iterator{after_last_valid_frame_}; } ValidFramesModifier::ValidFramesModifier(MultiFrame &multiframe) : multiframe_(multiframe) {} -ValidFramesModifier::Iterator ValidFramesModifier::begin() { return Iterator{&multiframe_.frames_[0], *this}; } +ValidFramesModifier::Iterator ValidFramesModifier::begin() { + if (multiframe_.frames_[0].IsValid()) { + return Iterator{&multiframe_.frames_[0], *this}; + } + return end(); +} + ValidFramesModifier::Iterator ValidFramesModifier::end() { return Iterator{multiframe_.frames_.data() + multiframe_.frames_.size(), *this}; } @@ -103,7 +115,12 @@ ValidFramesConsumer::~ValidFramesConsumer() noexcept { multiframe_.DefragmentValidFrames(); } -ValidFramesConsumer::Iterator ValidFramesConsumer::begin() { return Iterator{&multiframe_.frames_[0], *this}; } +ValidFramesConsumer::Iterator ValidFramesConsumer::begin() { + if (multiframe_.frames_[0].IsValid()) { + return Iterator{&multiframe_.frames_[0], *this}; + } + return end(); +} ValidFramesConsumer::Iterator ValidFramesConsumer::end() { return Iterator{multiframe_.frames_.data() + multiframe_.frames_.size(), *this}; From 50f76b926bac82ad0c0d3287a2cfc8d6e5435ada Mon Sep 17 00:00:00 2001 From: jeremy Date: Fri, 9 Dec 2022 12:20:01 +0100 Subject: [PATCH 2/2] Make MultiFrame pointer instead of ref inside impl of iterators --- src/query/v2/multiframe.cpp | 32 ++++++++++++++++---------------- src/query/v2/multiframe.hpp | 8 ++++---- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/src/query/v2/multiframe.cpp b/src/query/v2/multiframe.cpp index 27f9607d5..02d396fda 100644 --- a/src/query/v2/multiframe.cpp +++ b/src/query/v2/multiframe.cpp @@ -69,7 +69,7 @@ ValidFramesConsumer MultiFrame::GetValidFramesConsumer() { return ValidFramesCon InvalidFramesPopulator MultiFrame::GetInvalidFramesPopulator() { return InvalidFramesPopulator{*this}; } -ValidFramesReader::ValidFramesReader(MultiFrame &multiframe) : multiframe_(multiframe) { +ValidFramesReader::ValidFramesReader(MultiFrame &multiframe) : multiframe_(&multiframe) { /* From: https://en.cppreference.com/w/cpp/algorithm/find Returns an iterator to the first element in the range [first, last) that satisfies specific criteria: @@ -81,55 +81,55 @@ ValidFramesReader::ValidFramesReader(MultiFrame &multiframe) : multiframe_(multi */ auto it = std::find_if(multiframe.frames_.begin(), multiframe.frames_.end(), [](const auto &frame) { return !frame.IsValid(); }); - after_last_valid_frame_ = multiframe_.frames_.data() + std::distance(multiframe.frames_.begin(), it); + after_last_valid_frame_ = multiframe_->frames_.data() + std::distance(multiframe.frames_.begin(), it); } ValidFramesReader::Iterator ValidFramesReader::begin() { - if (multiframe_.frames_[0].IsValid()) { - return Iterator{&multiframe_.frames_[0]}; + if (multiframe_->frames_[0].IsValid()) { + return Iterator{&multiframe_->frames_[0]}; } return end(); } ValidFramesReader::Iterator ValidFramesReader::end() { return Iterator{after_last_valid_frame_}; } -ValidFramesModifier::ValidFramesModifier(MultiFrame &multiframe) : multiframe_(multiframe) {} +ValidFramesModifier::ValidFramesModifier(MultiFrame &multiframe) : multiframe_(&multiframe) {} ValidFramesModifier::Iterator ValidFramesModifier::begin() { - if (multiframe_.frames_[0].IsValid()) { - return Iterator{&multiframe_.frames_[0], *this}; + if (multiframe_->frames_[0].IsValid()) { + return Iterator{&multiframe_->frames_[0], *this}; } return end(); } ValidFramesModifier::Iterator ValidFramesModifier::end() { - return Iterator{multiframe_.frames_.data() + multiframe_.frames_.size(), *this}; + return Iterator{multiframe_->frames_.data() + multiframe_->frames_.size(), *this}; } -ValidFramesConsumer::ValidFramesConsumer(MultiFrame &multiframe) : multiframe_(multiframe) {} +ValidFramesConsumer::ValidFramesConsumer(MultiFrame &multiframe) : multiframe_(&multiframe) {} // NOLINTNEXTLINE (bugprone-exception-escape) ValidFramesConsumer::~ValidFramesConsumer() noexcept { // TODO Possible optimisation: only DefragmentValidFrames if one frame has been invalidated? Only if does not // cost too much to store it - multiframe_.DefragmentValidFrames(); + multiframe_->DefragmentValidFrames(); } ValidFramesConsumer::Iterator ValidFramesConsumer::begin() { - if (multiframe_.frames_[0].IsValid()) { - return Iterator{&multiframe_.frames_[0], *this}; + if (multiframe_->frames_[0].IsValid()) { + return Iterator{&multiframe_->frames_[0], *this}; } return end(); } ValidFramesConsumer::Iterator ValidFramesConsumer::end() { - return Iterator{multiframe_.frames_.data() + multiframe_.frames_.size(), *this}; + return Iterator{multiframe_->frames_.data() + multiframe_->frames_.size(), *this}; } -InvalidFramesPopulator::InvalidFramesPopulator(MultiFrame &multiframe) : multiframe_(multiframe) {} +InvalidFramesPopulator::InvalidFramesPopulator(MultiFrame &multiframe) : multiframe_(&multiframe) {} InvalidFramesPopulator::Iterator InvalidFramesPopulator::begin() { - for (auto &frame : multiframe_.frames_) { + for (auto &frame : multiframe_->frames_) { if (!frame.IsValid()) { return Iterator{&frame}; } @@ -138,7 +138,7 @@ InvalidFramesPopulator::Iterator InvalidFramesPopulator::begin() { } InvalidFramesPopulator::Iterator InvalidFramesPopulator::end() { - return Iterator{multiframe_.frames_.data() + multiframe_.frames_.size()}; + return Iterator{multiframe_->frames_.data() + multiframe_->frames_.size()}; } } // namespace memgraph::query::v2 diff --git a/src/query/v2/multiframe.hpp b/src/query/v2/multiframe.hpp index 0b6896422..8588993a7 100644 --- a/src/query/v2/multiframe.hpp +++ b/src/query/v2/multiframe.hpp @@ -137,7 +137,7 @@ class ValidFramesReader { private: FrameWithValidity *after_last_valid_frame_; - MultiFrame &multiframe_; + MultiFrame *multiframe_; }; class ValidFramesModifier { @@ -192,7 +192,7 @@ class ValidFramesModifier { Iterator end(); private: - MultiFrame &multiframe_; + MultiFrame *multiframe_; }; class ValidFramesConsumer { @@ -246,7 +246,7 @@ class ValidFramesConsumer { Iterator end(); private: - MultiFrame &multiframe_; + MultiFrame *multiframe_; }; class InvalidFramesPopulator { @@ -296,7 +296,7 @@ class InvalidFramesPopulator { Iterator end(); private: - MultiFrame &multiframe_; + MultiFrame *multiframe_; }; } // namespace memgraph::query::v2