Skip to content

Commit

Permalink
Address the remaining issues, so that this becomes ready for merge.
Browse files Browse the repository at this point in the history
Signed-off-by: Johannes Kalmbach <[email protected]>
  • Loading branch information
joka921 committed Feb 10, 2025
1 parent 2dba572 commit dd64d9d
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 16 deletions.
9 changes: 4 additions & 5 deletions src/engine/idTable/CompressedExternalIdTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,13 @@

namespace ad_utility {

namespace {
namespace compressedExternalIdTable::detail {
template <typename B, typename R>
CPP_requires(HasPushBackRequires, requires(B& b, const R& r)(b.push_back(r)));

template <typename B, typename R>
CPP_concept HasPushBack = CPP_requires_ref(HasPushBackRequires, B, R);
}
} // namespace compressedExternalIdTable::detail

using namespace ad_utility::memory_literals;

Expand Down Expand Up @@ -347,9 +347,8 @@ class CompressedExternalIdTableBase {
// Add a single row to the input. The type of `row` needs to be something that
// can be `push_back`ed to a `IdTable`.
CPP_template(typename R)(
requires HasPushBack<decltype(currentBlock_),
R>) void push(const R& row)
{
requires compressedExternalIdTable::detail::HasPushBack<
decltype(currentBlock_), R>) void push(const R& row) {
++numElementsPushed_;
currentBlock_.push_back(row);
if (currentBlock_.size() >= blocksize_) {
Expand Down
23 changes: 13 additions & 10 deletions src/engine/idTable/IdTable.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,23 +240,26 @@ class IdTable {
// `IdTables` are expensive to copy, so we disable accidental copies as they
// are most likely bugs. To explicitly copy an `IdTable`, the `clone()` member
// function (see below) can be used.
CPP_template(typename = void)(requires(!isView))
IdTable(const IdTable&) = delete;

CPP_template(typename = void)(requires(!isView)) IdTable& operator=(
const IdTable&) requires(!isView)
= delete;

// Note: We currently only disable the copy operations in C++20 mode.
// TODO<joka921> implement a facility (probably via inheritance) where we can
// also implement the deleted copy operations for C++17
#ifndef QLEVER_CPP_17
IdTable(const IdTable&) requires(!isView) = delete;
IdTable& operator=(const IdTable&) requires(!isView) = delete;

// Views are copyable, as they are cheap to copy.
IdTable(const IdTable&) requires isView = default;
IdTable& operator=(const IdTable&) requires isView = default;

// `IdTable`s are movable
IdTable(IdTable&& other) noexcept requires(!isView) = default;
IdTable& operator=(IdTable&& other) noexcept requires(!isView) = default;
#else
IdTable(const IdTable&) = default;
IdTable& operator=(const IdTable&) = default;
#endif

// `IdTable`s are movable
IdTable(IdTable&& other) noexcept = default;
IdTable& operator=(IdTable&& other) noexcept = default;

private:
// Make the other instantiations of `IdTable` friends to allow for conversion
// between them using a private interface.
Expand Down
2 changes: 1 addition & 1 deletion src/util/StringUtils.h
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ CPP_template(typename Range)(

// Similar to the overload of `lazyStrJoin` above, but the result is returned as
// a string.
CPP_template_def(typename Range)(
CPP_template(typename Range)(
requires ql::ranges::input_range<Range> CPP_and ad_utility::Streamable<
std::iter_reference_t<ql::ranges::iterator_t<Range>>>) std::string
lazyStrJoin(Range&& r, std::string_view separator);
Expand Down
2 changes: 2 additions & 0 deletions test/IdTableTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,8 @@ TEST(IdTable, staticAsserts) {
static_assert(std::is_trivially_copyable_v<IdTableStatic<1>::const_iterator>);
static_assert(ql::ranges::random_access_range<IdTable>);
static_assert(ql::ranges::random_access_range<IdTableStatic<1>>);

static_assert(std::is_copy_constructible_v<IdTableView<1>>);
}

TEST(IdTable, constructorsAreSfinaeFriendly) {
Expand Down

0 comments on commit dd64d9d

Please sign in to comment.