ci: Add clang-tidy check for &&, || and ! instead of and, or and not

.clang-tidy

@@ -1,4 +1,7 @@
 ---
-Checks: '-*,readability-container-size-empty,readability-implicit-bool-cast,readability-implicit-bool-conversion,modernize-use-equals-default,modernize-use-override,modernize-use-using,readability-braces-around-statements,modernize-use-nullptr,performance-move-const-arg,cppcoreguidelines-pro-type-member-init,cppcoreguidelines-init-variables,clang-analyzer-optin.cplusplus.UninitializedObject'
+Checks: '-*,readability-container-size-empty,readability-implicit-bool-cast,readability-implicit-bool-conversion,modernize-use-equals-default,modernize-use-override,modernize-use-using,readability-braces-around-statements,modernize-use-nullptr,performance-move-const-arg,cppcoreguidelines-pro-type-member-init,cppcoreguidelines-init-variables,clang-analyzer-optin.cplusplus.UninitializedObject,readability-operators-representation'
 HeaderFilterRegex: '.*(?<!nlohmann\/json)\.(hpp|cpp|ipp)$'
 AnalyzeTemporaryDtors: true
+CheckOptions:
+  readability-operators-representation.BinaryOperators: '&&;&=;&;|;~;!;!=;||;|=;^;^='
+  readability-operators-representation.OverloadedOperators: '&&;&=;&;|;~;!;!=;||;|=;^;^='

.gitlab-ci.yml

@@ -20,11 +20,11 @@ clang_tidy:
         apt-get update
         && apt-get install -y g++-8 libstdc++-8-dev parallel software-properties-common 
         && curl -L https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add
-        && add-apt-repository 'deb http://apt.llvm.org/focal/ llvm-toolchain-focal-16 main'
-        && apt-get install -y clang-16 clang-tidy-16
-        && ln -s /usr/bin/clang++-16 /usr/bin/clang++
-        && ln -s /usr/bin/clang-16 /usr/bin/clang
-        && ln -s /usr/bin/clang-tidy-16 /usr/bin/clang-tidy
+        && add-apt-repository 'deb http://apt.llvm.org/focal/ llvm-toolchain-focal-17 main'
+        && apt-get install -y clang-17 clang-tidy-17
+        && ln -s /usr/bin/clang++-17 /usr/bin/clang++
+        && ln -s /usr/bin/clang-17 /usr/bin/clang
+        && ln -s /usr/bin/clang-tidy-17 /usr/bin/clang-tidy
         && mkdir -p /opt/lib/gcc/x86_64-linux-gnu
         && ln -s /usr/lib/gcc/x86_64-linux-gnu/8/ /opt/lib/gcc/x86_64-linux-gnu/ 
         && clang++ --gcc-toolchain=/opt -v

Alignment/include/ActsAlignment/Kernel/Alignment.ipp

@@ -29,7 +29,7 @@ ActsAlignment::Alignment<fitter_t>::evaluateTrackAlignmentState(
   // Perform the fit
   auto fitRes = m_fitter.fit(begin, end, sParameters, fitOptions, tracks);
 
-  if (not fitRes.ok()) {
+  if (!fitRes.ok()) {
     ACTS_WARNING("Fit failure");
     return fitRes.error();
   }
@@ -89,7 +89,7 @@ void ActsAlignment::Alignment<fitter_t>::calculateAlignmentParameters(
     auto evaluateRes = evaluateTrackAlignmentState(
         fitOptions.geoContext, sourcelinks, sParameters,
         fitOptionsWithRefSurface, alignResult.idxedAlignSurfaces, alignMask);
-    if (not evaluateRes.ok()) {
+    if (!evaluateRes.ok()) {
       ACTS_DEBUG("Evaluation of alignment state for track " << iTraj
                                                             << " failed");
       continue;
@@ -200,7 +200,7 @@ ActsAlignment::Alignment<fitter_t>::updateAlignmentParameters(
                  << deltaAlignmentParam);
     bool updated = alignedTransformUpdater(alignedDetElements.at(index), gctx,
                                            newTransform);
-    if (not updated) {
+    if (!updated) {
       ACTS_ERROR("Update alignment parameters for detector element failed");
       return AlignmentError::AlignmentParametersUpdateFailure;
     }
@@ -289,15 +289,15 @@ ActsAlignment::Alignment<fitter_t>::align(
     auto updateRes = updateAlignmentParameters(
         alignOptions.fitOptions.geoContext, alignOptions.alignedDetElements,
         alignOptions.alignedTransformUpdater, alignResult);
-    if (not updateRes.ok()) {
+    if (!updateRes.ok()) {
       ACTS_ERROR("Update alignment parameters failed: " << updateRes.error());
       return updateRes.error();
     }
     alignmentParametersUpdated = true;
   }  // end of all iterations
 
   // Alignment failure if not converged
-  if (not converged) {
+  if (!converged) {
     ACTS_ERROR("Alignment is not converged.");
     alignResult.result = AlignmentError::ConvergeFailure;
   }

Alignment/include/ActsAlignment/Kernel/detail/AlignmentEngine.hpp

@@ -136,7 +136,7 @@ TrackAlignmentState trackAlignmentState(
 
     // Only measurement states matter (we can't align non-measurement states,
     // no?)
-    if (not ts.typeFlags().test(TrackStateFlag::MeasurementFlag)) {
+    if (!ts.typeFlags().test(TrackStateFlag::MeasurementFlag)) {
       return true;
     }
     // Check if the reference surface is to be aligned

Alignment/src/Kernel/detail/AlignmentEngine.cpp

@@ -13,22 +13,22 @@ namespace detail {
 
 void resetAlignmentDerivative(Acts::AlignmentToBoundMatrix& alignToBound,
                               AlignmentMask mask) {
-  if (not ACTS_CHECK_BIT(mask, AlignmentMask::Center0)) {
+  if (!ACTS_CHECK_BIT(mask, AlignmentMask::Center0)) {
     alignToBound.col(Acts::eAlignmentCenter0) = Acts::AlignmentVector::Zero();
   }
-  if (not ACTS_CHECK_BIT(mask, AlignmentMask::Center1)) {
+  if (!ACTS_CHECK_BIT(mask, AlignmentMask::Center1)) {
     alignToBound.col(Acts::eAlignmentCenter1) = Acts::AlignmentVector::Zero();
   }
-  if (not ACTS_CHECK_BIT(mask, AlignmentMask::Center2)) {
+  if (!ACTS_CHECK_BIT(mask, AlignmentMask::Center2)) {
     alignToBound.col(Acts::eAlignmentCenter2) = Acts::AlignmentVector::Zero();
   }
-  if (not ACTS_CHECK_BIT(mask, AlignmentMask::Rotation0)) {
+  if (!ACTS_CHECK_BIT(mask, AlignmentMask::Rotation0)) {
     alignToBound.col(Acts::eAlignmentRotation0) = Acts::AlignmentVector::Zero();
   }
-  if (not ACTS_CHECK_BIT(mask, AlignmentMask::Rotation1)) {
+  if (!ACTS_CHECK_BIT(mask, AlignmentMask::Rotation1)) {
     alignToBound.col(Acts::eAlignmentRotation1) = Acts::AlignmentVector::Zero();
   }
-  if (not ACTS_CHECK_BIT(mask, AlignmentMask::Rotation2)) {
+  if (!ACTS_CHECK_BIT(mask, AlignmentMask::Rotation2)) {
     alignToBound.col(Acts::eAlignmentRotation2) = Acts::AlignmentVector::Zero();
   }
 }

CI/clang_tidy/limits.yml

@@ -17,3 +17,4 @@ limits:
   "cppcoreguidelines-init-variables": 0
   "clang-analyzer-optin.cplusplus.UninitializedObject": 0
   "clang-diagnostic-error": 0
+  "readability-operators-representation": 0

CI/clang_tidy/run_clang_tidy.py

@@ -0,0 +1,169 @@
+#!/usr/bin/env python3
+import argparse
+from concurrent.futures import ThreadPoolExecutor, as_completed
+from multiprocessing import cpu_count
+import subprocess
+from subprocess import check_call, check_output, CalledProcessError
+from pathlib import Path
+import json
+import re
+import sys
+import os
+import threading
+
+import rich.console
+import rich.progress
+import rich.panel
+import rich.live
+import rich.text
+import rich.table
+import rich.rule
+import rich.spinner
+
+
+def which(cmd: str):
+    try:
+        return check_output(["command", "-v", cmd]).decode().strip()
+    except CalledProcessError:
+        return None
+
+
+def main():
+    p = argparse.ArgumentParser()
+    p.add_argument("--clang-tidy", default=which("clang-tidy"))
+    p.add_argument("--clang-format", default=which("clang-format"))
+    p.add_argument("--jobs", "-j", type=int, default=cpu_count())
+    p.add_argument("--fix", action="store_true")
+    p.add_argument("--include", action="append", default=[])
+    p.add_argument("--exclude", action="append", default=[])
+    p.add_argument("--ignore-compiler-errors", action="store_true")
+    p.add_argument("build", type=Path)
+    p.add_argument("source", type=Path)
+
+    args = p.parse_args()
+
+    assert args.clang_tidy is not None, "clang-tidy not found"
+    assert args.clang_format is not None, "clang-format not found"
+
+    args.include = [re.compile(f) for f in args.include]
+    args.exclude = [re.compile(f) for f in args.exclude]
+
+    check_call([args.clang_tidy, "--version"], stdout=subprocess.DEVNULL)
+    check_call([args.clang_format, "--version"], stdout=subprocess.DEVNULL)
+
+    assert (
+        args.build.exists() and args.build.is_dir()
+    ), f"{args.build} is not a directory"
+    assert (
+        args.source.exists() and args.source.is_dir()
+    ), f"{args.source} is not a directory"
+
+    with (args.build / "compile_commands.json").open() as f:
+        compilation_database = json.load(f)
+
+    futures = []
+    files = []
+    active_files = {}
+    active_file_lock = threading.Lock()
+
+    def run(file: Path):
+        with active_file_lock:
+            active_files[threading.current_thread().ident] = file
+        cmd = [args.clang_tidy, "-p", args.build, file]
+        if args.fix:
+            cmd.append("-fix")
+
+        try:
+            out = check_output(cmd, stderr=subprocess.STDOUT).decode().strip()
+            error = False
+        except CalledProcessError as e:
+            out = e.output.decode().strip()
+            if args.ignore_compiler_errors and "Found compiler error(s)." in out:
+                out = "Found compiler error(s)."
+                error = False
+            else:
+                error = True
+        finally:
+            with active_file_lock:
+                active_files[threading.current_thread().ident] = None
+        return file, out, error
+
+    for dirpath, _, filenames in os.walk(args.source):
+        dirpath = Path(dirpath)
+        for file in filenames:
+            file = dirpath / file
+            if (
+                file.suffix in (".hpp", ".cpp", ".ipp")
+                and (
+                    len(args.include) == 0
+                    or any(flt.match(str(file)) for flt in args.include)
+                )
+                and not any(flt.match(str(file)) for flt in args.exclude)
+            ):
+                files.append(file)
+
+    with ThreadPoolExecutor(args.jobs) as tp:
+        for file in files:
+            assert file.exists(), f"{file} does not exist"
+            futures.append(tp.submit(run, file))
+
+        error = False
+
+        console = rich.console.Console()
+
+        prog = rich.progress.Progress()
+        log = []
+
+        def make_display():
+            t = rich.table.Table.grid()
+            t.add_column()
+            t.add_column()
+            with active_file_lock:
+                for f in active_files.values():
+                    if f is None:
+                        t.add_row("")
+                    else:
+                        t.add_row(rich.spinner.Spinner("dots", style="green"), f" {f}")
+
+            ot = rich.table.Table.grid(expand=True)
+            ot.add_column(ratio=1)
+            ot.add_column(ratio=1)
+
+            def emoji(err):
+                return ":red_circle:" if err else ":green_circle:"
+
+            ot.add_row(
+                t,
+                rich.console.Group(
+                    *[f"{emoji(err)} {line}" for err, line in log[-args.jobs :]]
+                ),
+            )
+
+            return rich.console.Group(rich.rule.Rule(), ot, prog)
+
+        task = prog.add_task("Running clang-tidy", total=len(futures))
+
+        with rich.live.Live(
+            make_display(), console=console, refresh_per_second=20, transient=False
+        ) as live:
+            for f in as_completed(futures):
+                file, result, this_error = f.result()
+                log.append((this_error, file))
+                error = this_error or error
+                console.print(
+                    rich.panel.Panel(
+                        result, title=str(file), style="red" if this_error else ""
+                    )
+                )
+                prog.advance(task)
+                live.update(make_display())
+            live.refresh()
+
+        if error:
+            return 1
+        else:
+            return 0
+
+
+if __name__ == "__main__":
+    sys.exit(main())

Core/include/Acts/Detector/detail/IndexedGridFiller.hpp

@@ -68,7 +68,7 @@ std::set<typename grid_type::index_t> localIndices(
     throw std::runtime_error("IndexedSurfaceGridFiller: no query point given.");
   }
 
-  if (not expansion.empty() and expansion.size() != grid_type::DIM) {
+  if (!expansion.empty() && expansion.size() != grid_type::DIM) {
     throw std::runtime_error(
         "IndexedSurfaceGridFiller: wrong dimension of bin expansion given.");
   }
@@ -222,7 +222,7 @@ struct IndexedGridFiller {
     }
 
     // Assign the indices into all
-    if (not aToAll.empty()) {
+    if (!aToAll.empty()) {
       assignToAll(iGrid, aToAll);
     }
   }

Core/include/Acts/Digitization/DigitizationSourceLink.hpp

@@ -47,12 +47,12 @@ class DigitizationSourceLink final {
 
 inline bool operator==(const DigitizationSourceLink& lhs,
                        const DigitizationSourceLink& rhs) {
-  return (lhs.geometryId() == rhs.geometryId()) and
+  return (lhs.geometryId() == rhs.geometryId()) &&
          (lhs.indices() == rhs.indices());
 }
 inline bool operator!=(const DigitizationSourceLink& lhs,
                        const DigitizationSourceLink& rhs) {
-  return not(lhs == rhs);
+  return !(lhs == rhs);
 }
 
 }  // namespace Acts

Core/include/Acts/EventData/Charge.hpp

@@ -61,7 +61,7 @@ struct Neutral {
   ///
   /// This constructor is only provided to allow consistent construction.
   constexpr Neutral(float absQ) noexcept {
-    assert((absQ == 0) and "Input charge must be zero");
+    assert((absQ == 0) && "Input charge must be zero");
     (void)absQ;
   }
 
@@ -80,7 +80,7 @@ struct Neutral {
 
   template <typename P, typename Q>
   constexpr auto qOverP(P momentum, Q signedQ) const noexcept {
-    assert((signedQ != 0) and "charge must be 0");
+    assert((signedQ != 0) && "charge must be 0");
     (void)signedQ;
     return 1.0f / momentum;
   }
@@ -106,7 +106,7 @@ struct SinglyCharged {
   ///
   /// This constructor is only provided to allow consistent construction.
   constexpr SinglyCharged(float absQ) noexcept {
-    assert((absQ == UnitConstants::e) and "Input charge magnitude must be e");
+    assert((absQ == UnitConstants::e) && "Input charge magnitude must be e");
     (void)absQ;
   }
 
@@ -151,7 +151,7 @@ class NonNeutralCharge {
  public:
   /// Construct with the magnitude of the input charge.
   constexpr NonNeutralCharge(float absQ) noexcept : m_absQ{absQ} {
-    assert((0 < absQ) and "Input charge magnitude must be positive");
+    assert((0 < absQ) && "Input charge magnitude must be positive");
   }
   constexpr NonNeutralCharge(SinglyCharged /*unused*/) noexcept
       : m_absQ{UnitConstants::e} {}
@@ -200,7 +200,7 @@ class AnyCharge {
  public:
   /// Construct with the magnitude of the input charge.
   constexpr AnyCharge(float absQ) noexcept : m_absQ{absQ} {
-    assert((0 <= absQ) and "Input charge magnitude must be zero or positive");
+    assert((0 <= absQ) && "Input charge magnitude must be zero or positive");
   }
   constexpr AnyCharge(SinglyCharged /*unused*/) noexcept
       : m_absQ{UnitConstants::e} {}

Core/include/Acts/EventData/GenericBoundTrackParameters.hpp

@@ -136,7 +136,7 @@ class GenericBoundTrackParameters {
   const std::optional<CovarianceMatrix>& covariance() const { return m_cov; }
   /// Covariance matrix of the spatial impact parameters (i.e., of d0 and z0)
   std::optional<ActsSquareMatrix<2>> spatialImpactParameterCovariance() const {
-    if (not m_cov.has_value()) {
+    if (!m_cov.has_value()) {
       return std::nullopt;
     }
 
@@ -146,7 +146,7 @@ class GenericBoundTrackParameters {
   /// Covariance matrix of the spatial and temporal impact parameters (i.e., of
   /// d0, z0, and t)
   std::optional<ActsSquareMatrix<3>> impactParameterCovariance() const {
-    if (not m_cov.has_value()) {
+    if (!m_cov.has_value()) {
       return std::nullopt;
     }
 
@@ -278,14 +278,14 @@ class GenericBoundTrackParameters {
   ///   want and we might decided that we will remove this in the future.
   friend bool operator==(const GenericBoundTrackParameters& lhs,
                          const GenericBoundTrackParameters& rhs) {
-    return (lhs.m_params == rhs.m_params) and (lhs.m_cov == rhs.m_cov) and
-           (lhs.m_surface == rhs.m_surface) and
+    return (lhs.m_params == rhs.m_params) && (lhs.m_cov == rhs.m_cov) &&
+           (lhs.m_surface == rhs.m_surface) &&
            (lhs.m_particleHypothesis == rhs.m_particleHypothesis);
   }
   /// Compare two bound track parameters for bitwise in-equality.
   friend bool operator!=(const GenericBoundTrackParameters& lhs,
                          const GenericBoundTrackParameters& rhs) {
-    return not(lhs == rhs);
+    return !(lhs == rhs);
   }
   /// Print information to the output stream.
   friend std::ostream& operator<<(std::ostream& os,