Update the style check to Clang-Format 16 (ad-freiburg#941)

The previously used version (`clang-format 14`) did not format C++20 `requires` clauses correctly. In addition `clang-format-16` has the following additional configuration options which we now use:
* Enforce unix-style line breaks (`\n` vs `\r\n`)
* Enforce a newline at the end of each source file

.clang-format

@@ -2,4 +2,8 @@
 BasedOnStyle: Google
 DerivePointerAlignment: false
 PointerAlignment: Left
+LineEnding: LF
+InsertNewlineAtEOF: true
+RequiresClausePosition: SingleLine
+IndentRequiresClause: false
 ...

.github/workflows/format-check.yml

@@ -18,7 +18,12 @@ jobs:
     steps:
       - uses: actions/checkout@v3
       - name: Install dependencies
-        run: sudo apt-get install -y clang-format-14
+        run:  |
+            wget https://apt.llvm.org/llvm.sh
+            sudo chmod +x llvm.sh
+            sed   's/apt-key del/echo/' llvm.sh -iy
+            sudo ./llvm.sh 16
+            sudo apt install -y clang-format-16
 
       - name: Run the format checker
         run: ${{github.workspace}}/misc/format-check.sh

benchmark/infrastructure/Benchmark.h

@@ -78,8 +78,9 @@ class BenchmarkResults {
    *  with the needed parameters.
    */
   template <typename Function>
-  requires std::invocable<Function> ResultEntry& addMeasurement(
-      const std::string& descriptor, const Function& functionToMeasure) {
+  requires std::invocable<Function>
+  ResultEntry& addMeasurement(const std::string& descriptor,
+                              const Function& functionToMeasure) {
     return addEntryToContainerVector(singleMeasurements_, descriptor,
                                      functionToMeasure);
   }

benchmark/infrastructure/BenchmarkMeasurementContainer.h

@@ -81,8 +81,8 @@ class ResultEntry : public BenchmarkMetadataGetter {
   measured and saved.
   */
   template <typename Function>
-  requires std::invocable<Function> ResultEntry(
-      const std::string& descriptor, const Function& functionToMeasure)
+  requires std::invocable<Function>
+  ResultEntry(const std::string& descriptor, const Function& functionToMeasure)
       : descriptor_{descriptor},
         measuredTime_{measureTimeOfFunction(functionToMeasure)} {}
 
@@ -122,8 +122,9 @@ class ResultGroup : public BenchmarkMetadataGetter {
   measured and saved.
   */
   template <typename Function>
-  requires std::invocable<Function> ResultEntry& addMeasurement(
-      const std::string& descriptor, const Function& functionToMeasure) {
+  requires std::invocable<Function>
+  ResultEntry& addMeasurement(const std::string& descriptor,
+                              const Function& functionToMeasure) {
     entries_.push_back(ad_utility::make_copyable_unique<ResultEntry>(
         descriptor, functionToMeasure));
     return (*entries_.back());
@@ -204,8 +205,8 @@ class ResultTable : public BenchmarkMetadataGetter {
   @param row, column Which table entry to read. Starts with `(0,0)`.
   */
   template <typename T>
-  requires std::is_same_v<T, float> || std::is_same_v<T, std::string> T
-  getEntry(const size_t row, const size_t column) {
+  requires std::is_same_v<T, float> || std::is_same_v<T, std::string>
+  T getEntry(const size_t row, const size_t column) {
     // There is a chance, that the entry of the table does NOT have type T,
     // in which case this will cause an error. As this is a mistake on the
     // side of the user, we don't really care.

benchmark/infrastructure/BenchmarkToJson.cpp

@@ -27,10 +27,10 @@ Requires `TranslationFunction` to be a invocable function, that takes one
 parameter of a specific type and returns a `nlohmann::json` object.
 */
 template <typename SourceType, typename TranslationFunction>
-concept TransformsSourceTypeToJson = requires(TranslationFunction t,
-                                              SourceType v) {
-  { t(v) } -> std::same_as<nlohmann::json>;
-};
+concept TransformsSourceTypeToJson =
+    requires(TranslationFunction t, SourceType v) {
+      { t(v) } -> std::same_as<nlohmann::json>;
+    };
 
 /*
 @brief Transforms the content of a vector into a json array, using a

misc/format-check.sh

@@ -9,16 +9,16 @@ done <sourcelist
 
 ERROR=0
 for source in "${SOURCE_FILES[@]}" ;do
-	clang-format-14 -output-replacements-xml $source | grep "<replacement " &> /dev/null
+	clang-format-16 -output-replacements-xml $source | grep "<replacement " &> /dev/null
 	HAS_WRONG_FILES=$?
 	if [ $HAS_WRONG_FILES -ne 1 ] ; then
 		# Print an error and exit
 		printf "\x1b[31mError: "
 		printf "The source file \x1b[m$source\x1b[31m does not match the code style\n"
 		printf "Use clang-format with the .clang-format provided in the QLever\n"
 		printf "repository's root to ensure all code files are formatted "
-		printf "properly. We currently use clang-format 14\n"
-		printf "(can be installed as \"clang-format-14\" on Ubuntu 22.04.\n"
+		printf "properly. We currently use clang-format 16\n"
+		printf "(See `.github/workflows/format-check.yml` for instructions on how to install it.\n"
 		printf "\x1b[m"
 		ERROR=1
 	fi

src/engine/CallFixedSize.h

@@ -12,58 +12,65 @@
 #include "util/Exception.h"
 #include "util/Forward.h"
 
-// The functions and macros are able to call functions that take a set of integers as
-// template parameters with integers that are only known at runtime. To make this
-// work, the possible compile time integers have to be in a range [0, ..., MAX] where
-// MAX is a compile-time constant. For runtime integers that are > MAX, the function
-// is called with 0 as a compile-time parameter. This behavior is useful for the
-// `IdTables` (see `IdTable.h`) where `0` is a special value that means "the number of columns is
-// only known at runtime". Note that it is relatively easy to customize this behavior such that for
-// example integers that are > MAX lead to a runtime-error. This would make it possible to use these
-// facilities also for a "static switch"
+// The functions and macros are able to call functions that take a set of
+// integers as template parameters with integers that are only known at runtime.
+// To make this work, the possible compile time integers have to be in a range
+// [0, ..., MAX] where MAX is a compile-time constant. For runtime integers that
+// are > MAX, the function is called with 0 as a compile-time parameter. This
+// behavior is useful for the `IdTables` (see `IdTable.h`) where `0` is a
+// special value that means "the number of columns is only known at runtime".
+// Note that it is relatively easy to customize this behavior such that for
+// example integers that are > MAX lead to a runtime-error. This would make it
+// possible to use these facilities also for a "static switch"
 // TODO<joka921> Also implement such a behavior.
 
 // There are currently two possibilities to use this interface:
-// 1. Using the macros CALL_FIXED_SIZE_1, CALL_FIXED_SIZE_2 and CALL_FIXED_SIZE_3.
+// 1. Using the macros CALL_FIXED_SIZE_1, CALL_FIXED_SIZE_2 and
+// CALL_FIXED_SIZE_3.
 //    `CALL_FIXED_SIZE_1(i, func, additionalArguments...)` calls
 //     `std::invoke(func<f(i)>, additionalArguments...)` where f(i) = i
 //     <=DEFAULT_MAX_NUM_COLUMNS_STATIC_ID_TABLE? i : 0`
 //   `CALL_FIXED_SIZE_2` and `CALL_FIXED_SIZE_3` work similarly, but with 2 or 3
 //   integer arguments that are converted to template parameters.
-// 2. Using the templated functions `callFixedSize1`, `callFixedSize2` and `callFixedSize3`,
+// 2. Using the templated functions `callFixedSize1`, `callFixedSize2` and
+// `callFixedSize3`,
 //    that all live inside the `ad_utility` namespace.
 //    `callFixedSize1(i, lambda, args...)` calls
 //    `lambda(std::integer_constant<int, i'>, args...)`
 // The first version allows a convenient syntax for most cases.
 // The second version provides a macro-free typesafe interface that can be also
 // used in the corner cases where the first version fails. It also allows to
-// configure the maximal value, and it is easy to implement `callFixedSize4` etc.
-// for a higher number of parameters.
+// configure the maximal value, and it is easy to implement `callFixedSize4`
+// etc. for a higher number of parameters.
 
 // For simple examples that illustrate the possibilities and limitations of both
 // the macro and the template function interface, see `CallFixedSizeTest.cpp`.
 
 namespace ad_utility {
 namespace detail {
 
-// Internal helper functions that calls `lambda.template operator()<I, J,...)(args)`
-// where `I, J, ...` are the elements in the `array`. Requires that each element in
-// the `array` is `<= maxValue`.
+// Internal helper functions that calls `lambda.template operator()<I,
+// J,...)(args)` where `I, J, ...` are the elements in the `array`. Requires
+// that each element in the `array` is `<= maxValue`.
 template <int maxValue, size_t NumValues, std::integral Int>
-auto callLambdaForIntArray(std::array<Int, NumValues> array, auto&& lambda, auto&&... args) {
-  AD_CONTRACT_CHECK(std::ranges::all_of(array, [](auto el) { return el <= maxValue; }));
+auto callLambdaForIntArray(std::array<Int, NumValues> array, auto&& lambda,
+                           auto&&... args) {
+  AD_CONTRACT_CHECK(
+      std::ranges::all_of(array, [](auto el) { return el <= maxValue; }));
   using ArrayType = std::array<Int, NumValues>;
 
   // Call the `lambda` when the correct compile-time `Int`s are given as a
   // `std::integer_sequence`.
-  auto applyOnIntegerSequence = [&]<Int... Is>(std::integer_sequence<Int, Is...>) {
-    return lambda.template operator()<Is...>(AD_FWD(args)...);
-  };
+  auto applyOnIntegerSequence =
+      [&]<Int... Is>(std::integer_sequence<Int, Is...>) {
+        return lambda.template operator()<Is...>(AD_FWD(args)...);
+      };
 
   // We store the result of the actual computation in a `std::optional`.
   // If the `lambda` returns void we don't store anything, but we still need
   // a type for the `result` variable. We choose `int` as a dummy for this case.
-  using Result = decltype(applyOnIntegerSequence(ad_utility::toIntegerSequence<ArrayType{}>()));
+  using Result = decltype(applyOnIntegerSequence(
+      ad_utility::toIntegerSequence<ArrayType{}>()));
   static constexpr bool resultIsVoid = std::is_void_v<Result>;
   using Storage = std::conditional_t<resultIsVoid, int, std::optional<Result>>;
   Storage result;
@@ -83,15 +90,17 @@ auto callLambdaForIntArray(std::array<Int, NumValues> array, auto&& lambda, auto
 
   // Lambda: call `applyIf` for all the compile-time integers `Is...`. The
   // runtime parameter always is `array`.
-  auto f = [&applyIf]<ArrayType... Is>(std::integer_sequence<ArrayType, Is...>) {
-    (..., applyIf.template operator()<Is>());
-  };
+  auto f =
+      [&applyIf]<ArrayType... Is>(std::integer_sequence<ArrayType, Is...>) {
+        (..., applyIf.template operator()<Is>());
+      };
 
   // Call f for all combinations of compileTimeIntegers in `M x NumValues` where
   // `M` is `[0, ..., maxValue]` and `x` denotes the cartesian product of sets.
-  // Exactly one of these combinations is equal to `array`, and so the lambda will
-  // be executed exactly once.
-  f(ad_utility::cartesianPowerAsIntegerArray<static_cast<Int>(maxValue + 1), NumValues>());
+  // Exactly one of these combinations is equal to `array`, and so the lambda
+  // will be executed exactly once.
+  f(ad_utility::cartesianPowerAsIntegerArray<static_cast<Int>(maxValue + 1),
+                                             NumValues>());
 
   if constexpr (!resultIsVoid) {
     return std::move(result.value());
@@ -101,19 +110,24 @@ auto callLambdaForIntArray(std::array<Int, NumValues> array, auto&& lambda, auto
 // Overload for a single int.
 template <int maxValue, std::integral Int>
 auto callLambdaForIntArray(Int i, auto&& lambda, auto&&... args) {
-  return callLambdaForIntArray<maxValue>(std::array{i}, AD_FWD(lambda), AD_FWD(args)...);
+  return callLambdaForIntArray<maxValue>(std::array{i}, AD_FWD(lambda),
+                                         AD_FWD(args)...);
 }
 
 // The default function that maps `x` to the range `[0, ..., maxValue]`
-constexpr int mapToZeroIfTooLarge(int x, int maxValue) { return x <= maxValue ? x : 0; }
+constexpr int mapToZeroIfTooLarge(int x, int maxValue) {
+  return x <= maxValue ? x : 0;
 }
+}  // namespace detail
 
 // This function implements the main functionality.
 // It calls `functor(INT<f(ints[0])>, INT<f(ints[1])>, ..., args...)`
-// where `INT<N>` is `std::integral_constant<int, N>` and `f` is `mapToZeroIfTooLarge`.
-template <int MaxValue = DEFAULT_MAX_NUM_COLUMNS_STATIC_ID_TABLE, size_t NumIntegers,
-          std::integral Int>
-decltype(auto) callFixedSize(std::array<Int, NumIntegers> ints, auto&& functor, auto&&... args) {
+// where `INT<N>` is `std::integral_constant<int, N>` and `f` is
+// `mapToZeroIfTooLarge`.
+template <int MaxValue = DEFAULT_MAX_NUM_COLUMNS_STATIC_ID_TABLE,
+          size_t NumIntegers, std::integral Int>
+decltype(auto) callFixedSize(std::array<Int, NumIntegers> ints, auto&& functor,
+                             auto&&... args) {
   static_assert(NumIntegers > 0);
   // TODO<joka921, C++23> Use `std::bind_back`
   auto p = [](int i) { return detail::mapToZeroIfTooLarge(i, MaxValue); };
@@ -123,13 +137,16 @@ decltype(auto) callFixedSize(std::array<Int, NumIntegers> ints, auto&& functor,
   // is in the range `[0, (MaxValue +1)^ NumIntegers]` to a compile-time
   // value and to use this compile time constant on `applyOnSingleInteger`.
   // This can be done via a single call to `callLambdaForIntArray`.
-  return detail::callLambdaForIntArray<MaxValue>(ints, AD_FWD(functor), AD_FWD(args)...);
+  return detail::callLambdaForIntArray<MaxValue>(ints, AD_FWD(functor),
+                                                 AD_FWD(args)...);
 }
 
 // Overload for a single integer.
-template <int MaxValue = DEFAULT_MAX_NUM_COLUMNS_STATIC_ID_TABLE, std::integral Int>
+template <int MaxValue = DEFAULT_MAX_NUM_COLUMNS_STATIC_ID_TABLE,
+          std::integral Int>
 decltype(auto) callFixedSize(Int i, auto&& functor, auto&&... args) {
-  return callFixedSize<MaxValue>(std::array{i}, AD_FWD(functor), AD_FWD(args)...);
+  return callFixedSize<MaxValue>(std::array{i}, AD_FWD(functor),
+                                 AD_FWD(args)...);
 }
 
 }  // namespace ad_utility
@@ -144,8 +161,8 @@ decltype(auto) callFixedSize(Int i, auto&& functor, auto&&... args) {
 // `CALL_FIXED_SIZE(std::array(1, 2), ...`. For a single integer you can also
 // simply write `CALL_FIXED_SIZE(1, function, params)`, this is handled by
 // the corresponding overload of `ad_utility::callFixedSize`.
-#define CALL_FIXED_SIZE(integers, func, ...)                    \
-  ad_utility::callFixedSize(                                    \
-      integers, []<int... Is>(auto&&... args)->decltype(auto) { \
-        return std::invoke(func<Is...>, AD_FWD(args)...);       \
+#define CALL_FIXED_SIZE(integers, func, ...)                      \
+  ad_utility::callFixedSize(                                      \
+      integers, []<int... Is>(auto&&... args) -> decltype(auto) { \
+        return std::invoke(func<Is...>, AD_FWD(args)...);         \
       } __VA_OPT__(, ) __VA_ARGS__)

src/engine/CountAvailablePredicates.cpp

@@ -221,10 +221,10 @@ void CountAvailablePredicates::computePatternTrick(
 
   // declare openmp reductions which aggregate Hashmaps by adding the values for
   // corresponding keys
-#pragma omp declare reduction(MergeHashmapsId:MergeableHashMap<Id> \
-                              : omp_out %= omp_in)
-#pragma omp declare reduction(MergeHashmapsSizeT:MergeableHashMap<size_t> \
-                              : omp_out %= omp_in)
+#pragma omp declare reduction( \
+        MergeHashmapsId : MergeableHashMap<Id> : omp_out %= omp_in)
+#pragma omp declare reduction( \
+        MergeHashmapsSizeT : MergeableHashMap<size_t> : omp_out %= omp_in)
 
   // These variables are used to gather additional statistics
   size_t numEntitiesWithPatterns = 0;
@@ -236,8 +236,12 @@ void CountAvailablePredicates::computePatternTrick(
   if (input.size() > 0) {  // avoid strange OpenMP segfaults on GCC
 #pragma omp parallel
 #pragma omp single
-#pragma omp taskloop grainsize(500000) default(none) reduction(MergeHashmapsId:predicateCounts) reduction(MergeHashmapsSizeT : patternCounts) \
-                                       reduction(+ : numEntitiesWithPatterns) reduction(+: numPatternPredicates) reduction(+: numListPredicates) shared(input, subjectColumn, hasPattern, hasPredicate)
+#pragma omp taskloop grainsize(500000) default(none)                           \
+    reduction(MergeHashmapsId : predicateCounts)                               \
+    reduction(MergeHashmapsSizeT : patternCounts)                              \
+    reduction(+ : numEntitiesWithPatterns) reduction(+ : numPatternPredicates) \
+    reduction(+ : numListPredicates)                                           \
+    shared(input, subjectColumn, hasPattern, hasPredicate)
     for (size_t inputIdx = 0; inputIdx < input.size(); ++inputIdx) {
       // Skip over elements with the same subject (don't count them twice)
       Id subjectId = input(inputIdx, subjectColumn);
@@ -293,8 +297,11 @@ void CountAvailablePredicates::computePatternTrick(
       patternVec.end()) {  // avoid segfaults with OpenMP on GCC
 #pragma omp parallel
 #pragma omp single
-#pragma omp taskloop grainsize(100000) default(none) reduction(MergeHashmapsId:predicateCounts) reduction(+ : numPredicatesSubsumedInPatterns) \
-                                       reduction(+ : numEntitiesWithPatterns) reduction(+: numPatternPredicates) reduction(+: numListPredicates) shared( patternVec, patterns)
+#pragma omp taskloop grainsize(100000) default(none)                           \
+    reduction(MergeHashmapsId : predicateCounts)                               \
+    reduction(+ : numPredicatesSubsumedInPatterns)                             \
+    reduction(+ : numEntitiesWithPatterns) reduction(+ : numPatternPredicates) \
+    reduction(+ : numListPredicates) shared(patternVec, patterns)
     // TODO<joka921> When we use iterators (`patternVec.begin()`) for the loop,
     // there is a strange warning on clang15 when OpenMP is activated. Find out
     // whether this is a known issue and whether this will be fixed in later

src/engine/QueryPlanningCostFactors.h

@@ -20,4 +20,4 @@ class QueryPlanningCostFactors {
 
  private:
   ad_utility::HashMap<string, double> _factors;
-};
+};