In the middle of a review....

ad-freiburg · Jul 11, 2024 · 62a0800 · 62a0800
1 parent aa2bfad
commit 62a0800
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 30 deletions.
diff --git a/src/engine/LocalVocab.cpp b/src/engine/LocalVocab.cpp
@@ -81,16 +81,18 @@ std::vector<LocalVocab::LiteralOrIri> LocalVocab::getAllWordsForTesting()
 
 // TODO<joka921> Consider moving the cheap case (if precomputed) into the
 // header.
-auto LocalVocabEntry::lowerBoundInIndex() const -> BoundsInIndex {
+auto LocalVocabEntry::positionInVocab() const -> PositionInVocab {
   if (positionInVocabKnown) {
-    return {lowerBoundInIndex_, upperBoundInIndex_};
+    return {lowerBoundInVocab_, upperBoundInVocab_};
   }
   const IndexImpl& index = IndexImpl::staticGlobalSingletonIndex();
-  BoundsInIndex result;
+  PositionInVocab positionInVocab;
   const auto& vocab = index.getVocab();
-  result.lowerBound_ = vocab.lower_bound_external(toStringRepresentation());
-  result.upperBound_ = vocab.upper_bound_external(toStringRepresentation());
-  lowerBoundInIndex_ = result.lowerBound_;
-  upperBoundInIndex_ = result.upperBound_;
-  return result;
+  positionInVocab.lowerBound_ =
+      vocab.lower_bound_external(toStringRepresentation());
+  positionInVocab.upperBound_ =
+      vocab.upper_bound_external(toStringRepresentation());
+  lowerBoundInVocab_ = positionInVocab.lowerBound_;
+  upperBoundInVocab_ = positionInVocab.upperBound_;
+  return positionInVocab;
 }
diff --git a/src/engine/sparqlExpressions/RelationalExpressionHelpers.h b/src/engine/sparqlExpressions/RelationalExpressionHelpers.h
@@ -145,16 +145,6 @@ auto makeValueId(const S& value, const EvaluationContext* context) {
     auto visitor = [context](const auto& x) {
       auto res = makeValueId(x, context);
       return res;
-      /*
-      if constexpr (ad_utility::isSimilar<decltype(res), Id>) {
-        // We need the same return type on all cases when visiting a variant, so
-        // we need to return a pair here. As the second element is an upper
-        // bound, we have to increment it by one.
-        return std::pair{res, ValueId::fromBits(res.getBits() + 1)};
-      } else {
-        return res;
-      }
-       */
     };
     return std::visit(visitor, value);
 

diff --git a/src/global/IndexTypes.h b/src/global/IndexTypes.h
@@ -33,17 +33,17 @@ class alignas(16) LocalVocabEntry
     : public ad_utility::triple_component::LiteralOrIri {
  private:
   using Base = ad_utility::triple_component::LiteralOrIri;
-  mutable CopyableAtomic<VocabIndex> lowerBoundInIndex_;
-  mutable CopyableAtomic<VocabIndex> upperBoundInIndex_;
+  mutable CopyableAtomic<VocabIndex> lowerBoundInVocab_;
+  mutable CopyableAtomic<VocabIndex> upperBoundInVocab_;
   mutable CopyableAtomic<bool> positionInVocabKnown = false;
 
  public:
-  struct BoundsInIndex {
+  struct PositionInVocab {
     VocabIndex lowerBound_;
     VocabIndex upperBound_;
   };
   using Base::Base;
-  BoundsInIndex lowerBoundInIndex() const;
+  PositionInVocab positionInVocab() const;
 
   LocalVocabEntry(const Base& base) : Base{base} {}
   LocalVocabEntry(Base&& base) noexcept : Base{std::move(base)} {}

diff --git a/src/global/ValueId.h b/src/global/ValueId.h
@@ -145,9 +145,10 @@ class ValueId {
     if (type == LocalVocabIndex && otherType == LocalVocabIndex) [[unlikely]] {
       return *getLocalVocabIndex() <=> *other.getLocalVocabIndex();
     }
-    auto compareVocabAndLocalVocab = [](::VocabIndex vocabIndex,
-                                        ::LocalVocabIndex localVocabIndex) {
-      auto [lowerBound, upperBound] = localVocabIndex->lowerBoundInIndex();
+    auto compareVocabAndLocalVocab =
+        [](::VocabIndex vocabIndex,
+           ::LocalVocabIndex localVocabIndex) -> std::strong_ordering {
+      auto [lowerBound, upperBound] = localVocabIndex->positionInVocab();
       if (vocabIndex < lowerBound) {
         return std::strong_ordering::less;
       } else if (vocabIndex >= upperBound) {

diff --git a/src/global/ValueIdComparators.h b/src/global/ValueIdComparators.h
@@ -118,6 +118,7 @@ inline std::pair<RandomIt, RandomIt> getRangeForDatatype(RandomIt begin,
   if (ad_utility::contains(
           std::array{Datatype::LocalVocabIndex, Datatype::VocabIndex},
           datatype)) {
+    // TODO<joka921> This doesn't work....
     auto lower_bound =
         std::lower_bound(begin, end, Datatype::VocabIndex, comparator);
     auto upper_bound =
@@ -426,11 +427,11 @@ inline std::vector<std::pair<RandomIt, RandomIt>> getRangesForEqualIds(
   AD_CONTRACT_CHECK(valueIdBegin <= valueIdEnd);
   // This lambda enforces the invariants `non-empty` and `sorted` and also
   // merges directly adjacent ranges.
-  auto typeA = valueIdBegin.getDatatype();
-  auto typeB = valueIdEnd.getDatatype();
-  AD_CONTRACT_CHECK(typeA == typeB ||
-                    (ad_utility::contains(Id::stringTypes_, typeA) &&
-                     ad_utility::contains(Id::stringTypes_, typeB)));
+  auto typeBegin = valueIdBegin.getDatatype();
+  auto typeEnd = valueIdEnd.getDatatype();
+  AD_CONTRACT_CHECK(typeBegin == typeEnd ||
+                    (ad_utility::contains(Id::stringTypes_, typeBegin) &&
+                     ad_utility::contains(Id::stringTypes_, typeEnd)));
   switch (valueIdBegin.getDatatype()) {
     case Datatype::Double:
     case Datatype::Int: