Adding Static Analysis and other checks to clang-tidy

bootstrap.ps1

@@ -19,7 +19,7 @@ Push-Location (Join-Path $PSScriptRoot "./src/Simulation/qdk_sim_rs")
         --version $Env:NUGET_VERSION;
 Pop-Location
 
-if (-not (Test-Path Env:AGENT_OS)) {                                    # If not CI build, i.e. local build (if AGENT_OS envvar is not defined)
+if (-not (Test-Path Env:/AGENT_OS)) {                                    # If not CI build, i.e. local build (if AGENT_OS envvar is not defined)
     if ($Env:ENABLE_NATIVE -ne "false") {
         Write-Host "Build release flavor of the native simulator"
         $Env:BUILD_CONFIGURATION = "Release"

src/Qir/.clang-tidy

@@ -1,5 +1,11 @@
 Checks: 
-  '-*,bugprone-*,-readability-*,readability-identifier-*,readability-braces-around-statements'
+  'bugprone-*,readability-identifier-*,readability-braces-around-statements,cert*,\
+  -llvmlibc-callee-namespace,-llvmlibc-implementation-in-namespace,\
+  -llvmlibc-restrict-system-libc-headers,-modernize-use-trailing-return-type,\
+  -fuchsia-default-arguments-calls,-fuchsia-default-arguments-declarations,
+  -google-readability-casting'
+# TODO(rokuzmin): '*, . . .'
+
 WarningsAsErrors: '*'
 HeaderFilterRegex: '.*'
 

src/Qir/Runtime/lib/QIR/QubitManager.cpp

@@ -181,8 +181,8 @@ namespace Quantum
         sharedQubitStatusArray = new QubitIdType[(size_t)qubitCapacity];
 
         // These objects are passed by value (copies are created)
-        QubitListInSharedArray FreeQubitsFresh(0, qubitCapacity - 1, sharedQubitStatusArray);
-        RestrictedReuseArea outermostArea(FreeQubitsFresh);
+        QubitListInSharedArray freeQubitsFresh(0, qubitCapacity - 1, sharedQubitStatusArray);
+        RestrictedReuseArea outermostArea(freeQubitsFresh);
         freeQubitsInAreas.PushToBack(outermostArea);
 
         freeQubitCount = qubitCapacity;
@@ -431,7 +431,7 @@ namespace Quantum
         // existing values (NonMarker or indexes in the array).
 
         // Prepare new shared status array
-        QubitIdType* newStatusArray = new QubitIdType[(size_t)requestedCapacity];
+        auto* newStatusArray = new QubitIdType[(size_t)requestedCapacity];
         memcpy(newStatusArray, sharedQubitStatusArray, (size_t)qubitCapacity * sizeof(newStatusArray[0]));
         QubitListInSharedArray newFreeQubits(qubitCapacity, requestedCapacity - 1, newStatusArray);
 

src/Qir/Runtime/lib/QIR/arrays.cpp

@@ -54,8 +54,8 @@ int QirArray::Release()
     return rc;
 }
 
-QirArray::QirArray(TItemCount qubits_count)
-    : count(qubits_count)
+QirArray::QirArray(TItemCount qubitsCount)
+    : count(qubitsCount)
     , itemSizeInBytes((TItemSize)sizeof(void*))
     , ownsQubits(true)
     , refCount(1)
@@ -81,25 +81,25 @@ QirArray::QirArray(TItemCount qubits_count)
     }
 }
 
-QirArray::QirArray(TItemCount count_items, TItemSize item_size_bytes, TDimCount dimCount, TDimContainer&& dimSizes)
-    : count(count_items)
+QirArray::QirArray(TItemCount countItems, TItemSize itemSizeBytes, TDimCount dimCount, TDimContainer&& dimSizes)
+    : count(countItems)
 
     // Each array item needs to be properly aligned. Let's align them by correcting the `itemSizeInBytes`.
     , itemSizeInBytes(
-          ((item_size_bytes == 1) || (item_size_bytes == 2) || (item_size_bytes == 4) ||
-           ((item_size_bytes % sizeof(size_t)) == 0) // For built-in types or multiples of architecture alignment
+          ((itemSizeBytes == 1) || (itemSizeBytes == 2) || (itemSizeBytes == 4) ||
+           ((itemSizeBytes % sizeof(size_t)) == 0) // For built-in types or multiples of architecture alignment
            )
-              ? item_size_bytes // leave their natural alignment.
-                                // Other types align on the architecture boundary `sizeof(size_t)`:
-                                // 4 bytes on 32-bit arch, 8 on 64-bit arch.
-              : item_size_bytes + sizeof(size_t) - (item_size_bytes % sizeof(size_t)))
+              ? itemSizeBytes // leave their natural alignment.
+                              // Other types align on the architecture boundary `sizeof(size_t)`:
+                              // 4 bytes on 32-bit arch, 8 on 64-bit arch.
+              : itemSizeBytes + sizeof(size_t) - (itemSizeBytes % sizeof(size_t)))
 
     , dimensions(dimCount)
     , dimensionSizes(std::move(dimSizes))
     , ownsQubits(false)
     , refCount(1)
 {
-    assert(item_size_bytes != 0);
+    assert(itemSizeBytes != 0);
     assert(dimCount > 0);
 
     if (GlobalContext() != nullptr)
@@ -112,18 +112,18 @@ QirArray::QirArray(TItemCount count_items, TItemSize item_size_bytes, TDimCount
         assert(this->dimensionSizes.empty() || this->dimensionSizes[0] == this->count);
         if (this->dimensionSizes.empty())
         {
-            this->dimensionSizes.push_back(count_items);
+            this->dimensionSizes.push_back(countItems);
         }
     }
 
     assert(this->count * (TBufSize)itemSizeInBytes < std::numeric_limits<TBufSize>::max());
     // Using `<` rather than `<=` to calm down the compiler on 32-bit arch.
-    const TBufSize buffer_size = this->count * itemSizeInBytes;
-    if (buffer_size > 0)
+    const TBufSize bufferSize = this->count * itemSizeInBytes;
+    if (bufferSize > 0)
     {
-        this->buffer = new char[buffer_size];
-        assert(buffer_size <= std::numeric_limits<size_t>::max());
-        memset(this->buffer, 0, (size_t)buffer_size);
+        this->buffer = new char[bufferSize];
+        assert(bufferSize <= std::numeric_limits<size_t>::max());
+        memset(this->buffer, 0, (size_t)bufferSize);
     }
     else
     {
@@ -178,26 +178,26 @@ void QirArray::Append(const QirArray* other)
 
     assert((TBufSize)(other->count) * other->itemSizeInBytes < std::numeric_limits<TBufSize>::max());
     // Using `<` rather than `<=` to calm down the compiler on 32-bit arch.
-    const TBufSize other_size = other->count * other->itemSizeInBytes;
+    const TBufSize otherSize = other->count * other->itemSizeInBytes;
 
-    if (other_size == 0)
+    if (otherSize == 0)
     {
         return;
     }
 
     assert((TBufSize)(this->count) * this->itemSizeInBytes < std::numeric_limits<TBufSize>::max());
     // Using `<` rather than `<=` to calm down the compiler on 32-bit arch.
-    const TBufSize this_size = this->count * this->itemSizeInBytes;
+    const TBufSize thisSize = this->count * this->itemSizeInBytes;
 
-    char* new_buffer = new char[this_size + other_size];
-    if (this_size)
+    char* newBuffer = new char[thisSize + otherSize];
+    if (thisSize)
     {
-        memcpy(new_buffer, this->buffer, this_size);
+        memcpy(newBuffer, this->buffer, thisSize);
     }
-    memcpy(&new_buffer[this_size], other->buffer, other_size);
+    memcpy(&newBuffer[thisSize], other->buffer, otherSize);
 
     delete[] this->buffer;
-    this->buffer = new_buffer;
+    this->buffer = newBuffer;
     this->count += other->count;
     this->dimensionSizes[0] = this->count;
 }
@@ -279,11 +279,10 @@ extern "C"
         __quantum__rt__qubit_release_array(qa);
     }
 
-    // TODO: Use `QirArray::TItemSize itemSizeInBytes, QirArray::TItemCount count_items` (breaking change):
-    QirArray* __quantum__rt__array_create_1d(int32_t itemSizeInBytes, int64_t count_items)
+    QirArray* __quantum__rt__array_create_1d(int32_t itemSizeInBytes, int64_t countItems)
     {
         assert(itemSizeInBytes > 0);
-        return new QirArray((QirArray::TItemCount)count_items, (QirArray::TItemSize)itemSizeInBytes);
+        return new QirArray((QirArray::TItemCount)countItems, (QirArray::TItemSize)itemSizeInBytes);
     }
 
     // Bucketing of addref/release is non-standard so for now we'll keep the more traditional addref/release semantics
@@ -560,6 +559,10 @@ extern "C"
         const QirArray::TItemCount sliceItemsCount = std::accumulate(
             sliceDims.begin(), sliceDims.end(), (QirArray::TItemCount)1, std::multiplies<QirArray::TItemCount>());
         QirArray* slice = new QirArray(sliceItemsCount, itemSizeInBytes, dimensions, std::move(sliceDims));
+        if (nullptr == slice->buffer)
+        {
+            return slice;
+        }
         const QirArray::TItemCount singleIndexRunCount = RunCount(array->dimensionSizes, (QirArray::TDimCount)dim);
         const QirArray::TItemCount rowCount            = singleIndexRunCount * array->dimensionSizes[(size_t)dim];
 
@@ -636,13 +639,18 @@ extern "C"
         const QirArray::TItemCount projectItemsCount = std::accumulate(
             projectDims.begin(), projectDims.end(), (QirArray::TItemCount)1, std::multiplies<QirArray::TItemCount>());
         QirArray* project = new QirArray(projectItemsCount, itemSizeInBytes, dimensions - 1, std::move(projectDims));
+        if (nullptr == project->buffer)
+        {
+            return project;
+        }
 
         const QirArray::TItemCount singleIndexRunCount = RunCount(array->dimensionSizes, (QirArray::TDimCount)dim);
         const QirArray::TItemCount rowCount            = singleIndexRunCount * array->dimensionSizes[(size_t)dim];
 
         assert((QirArray::TBufSize)singleIndexRunCount * itemSizeInBytes <
                std::numeric_limits<QirArray::TBufSize>::max());
         // Using `<` rather than `<=` to calm down the compiler on 32-bit arch.
+
         const QirArray::TBufSize chunkSize = singleIndexRunCount * itemSizeInBytes;
 
         QirArray::TItemCount dst = 0;

src/Qir/Runtime/lib/QIR/callables.cpp

@@ -102,7 +102,11 @@ extern "C"
         {
             for (int i = increment; i < 0; i++)
             {
-                (void)callable->Release();
+                if (0 == callable->Release())
+                {
+                    assert(-1 == i && "Attempting to decrement reference count below zero!");
+                    break;
+                }
             }
         }
     }

src/Qir/Runtime/lib/QIR/strings.cpp

@@ -28,7 +28,7 @@ static QirString* CreateOrReuseAlreadyAllocated(std::string&& str)
     if (alreadyAllocated != AllocatedStrings().end())
     {
         qstr = alreadyAllocated->second;
-        assert(qstr->str.compare(str) == 0);
+        assert(qstr->str == str);
         qstr->refCount++;
     }
     else
@@ -95,7 +95,7 @@ extern "C"
     // Returns true if the two strings are equal, false otherwise.
     bool __quantum__rt__string_equal(QirString* left, QirString* right) // NOLINT
     {
-        assert((left == right) == (left->str.compare(right->str) == 0));
+        assert((left == right) == (left->str == right->str));
         return left == right;
     }
 
@@ -115,7 +115,7 @@ extern "C"
 
         // Remove padding zeros from the decimal part (relies on the fact that the output for integers always contains
         // period).
-        std::size_t pos1 = str.find_last_not_of("0");
+        std::size_t pos1 = str.find_last_not_of('0');
         if (pos1 != std::string::npos)
         {
             str.erase(pos1 + 1);

src/Qir/Runtime/prerequisites.ps1

@@ -4,7 +4,7 @@
 #Requires -Version 6.0
 
 if ($Env:ENABLE_QIRRUNTIME -ne "false") {
-    if (($IsWindows) -or ((Test-Path Env:AGENT_OS) -and ($Env:AGENT_OS.StartsWith("Win")))) {
+    if (($IsWindows) -or ((Test-Path Env:/AGENT_OS) -and ($Env:AGENT_OS.StartsWith("Win")))) {
         if (!(Get-Command clang        -ErrorAction SilentlyContinue) -or `
             !(Get-Command clang-format -ErrorAction SilentlyContinue)) {
             choco install llvm --version=11.1.0
@@ -29,10 +29,10 @@ if ($Env:ENABLE_QIRRUNTIME -ne "false") {
     } else {
         if (Get-Command sudo -ErrorAction SilentlyContinue) {
             sudo apt update
-            sudo apt-get install -y ninja-build clang-11 clang-tidy-11
+            sudo apt-get install -y ninja-build clang-11 clang-tidy-11 clang-format-11
         } else {
             apt update
-            apt-get install -y ninja-build clang-11 clang-tidy-11
+            apt-get install -y ninja-build clang-11 clang-tidy-11 clang-format-11
         }
     }
 }

src/Qir/Runtime/public/CoreTypes.hpp

@@ -36,7 +36,7 @@ class QUBIT;
 typedef QUBIT* Qubit; // Not a pointer to a memory location, just an integer - qubit id.
 
 class RESULT;
-typedef RESULT* Result; // TODO: Replace with `typedef uintXX_t Result`, where XX is 8|16|32|64.
+typedef RESULT* Result; // TODO(rokuzmin): Replace with `typedef uintXX_t Result`, where XX is 8|16|32|64.
                         //       Remove all the `RESULT`.
 
 enum ResultValue

src/Qir/Runtime/public/OutputStream.hpp

@@ -15,9 +15,15 @@ namespace Quantum
     {
         struct QIR_SHARED_API ScopedRedirector
         {
-            ScopedRedirector(std::ostream& newOstream);
+            explicit ScopedRedirector(std::ostream& newOstream);
             ~ScopedRedirector();
 
+          private:
+            ScopedRedirector(const ScopedRedirector&) = delete;
+            ScopedRedirector& operator=(const ScopedRedirector&) = delete;
+            ScopedRedirector(ScopedRedirector&&)                 = delete;
+            ScopedRedirector& operator=(ScopedRedirector&&) = delete;
+
           private:
             std::ostream& old;
         };

src/Qir/Runtime/public/QirRuntime.hpp

@@ -4,7 +4,7 @@
 #pragma once
 
 #include <cstdint>
-#include <stdarg.h> // for va_list
+#include <cstdarg> // for va_list
 
 #include "CoreTypes.hpp"
 #include "QirTypes.hpp"
@@ -310,7 +310,7 @@ extern "C"
     // TODO QIR_SHARED_API bool __quantum__rt__bigint_greater_eq(QirBigInt*, QirBigInt*); // NOLINT
 }
 
-// TODO: Consider separating the `extern "C"` exports and C++ exports.
+// TODO(rokuzmin): Consider separating the `extern "C"` exports and C++ exports.
 namespace Microsoft // Replace with `namespace Microsoft::Quantum` after migration to C++17.
 {
 namespace Quantum

src/Qir/Runtime/public/QirTypes.hpp

@@ -43,13 +43,13 @@ struct QIR_SHARED_API QirArray
     int AddRef();
     int Release();
 
-    QirArray(TItemCount cQubits);
+    explicit QirArray(TItemCount cQubits);
     QirArray(TItemCount cItems, TItemSize itemSizeInBytes, TDimCount dimCount = 1, TDimContainer&& dimSizes = {});
     QirArray(const QirArray& other);
 
     ~QirArray();
 
-    char* GetItemPointer(TItemCount index) const;
+    [[nodiscard]] char* GetItemPointer(TItemCount index) const;
     void Append(const QirArray* other);
 };
 
@@ -61,8 +61,8 @@ struct QIR_SHARED_API QirString
     long refCount = 1;
     std::string str;
 
-    QirString(std::string&& str);
-    QirString(const char* cstr);
+    explicit QirString(std::string&& str);
+    explicit QirString(const char* cstr);
 };
 
 /*======================================================================================================================
@@ -72,10 +72,10 @@ struct QIR_SHARED_API QirString
     a header that contains the relevant data. The header immediately precedes the tuple's buffer in memory when the
     tuple is created.
 ======================================================================================================================*/
-// TODO: Move these types to inside of `QirTupleHeader`.
+// TODO (rokuzmin): Move these types to inside of `QirTupleHeader`.
 using PTuplePointedType = uint8_t;
-using PTuple = PTuplePointedType*; // TODO: consider replacing `uint8_t*` with `void*` in order to block the accidental
-                                   //       {dereferencing and pointer arithmtic}.
+using PTuple = PTuplePointedType*; // TODO(rokuzmin): consider replacing `uint8_t*` with `void*` in order to block
+                                   //       the accidental {dereferencing and pointer arithmetic}.
                                    //       Much pointer arithmetic in tests. GetHeader() uses the pointer arithmetic.
 struct QIR_SHARED_API QirTupleHeader
 {
@@ -90,7 +90,7 @@ struct QIR_SHARED_API QirTupleHeader
 
     PTuple AsTuple()
     {
-        return data;
+        return (PTuple)data;
     }
 
     int AddRef();
@@ -139,12 +139,12 @@ static_assert(sizeof(TupleWithControls) == 2 * sizeof(void*),
 /*======================================================================================================================
     QirCallable
 ======================================================================================================================*/
-typedef void (*t_CallableEntry)(PTuple, PTuple, PTuple); // TODO: Move to `QirCallable::t_CallableEntry`.
-typedef void (*t_CaptureCallback)(PTuple, int32_t);      // TODO: Move to `QirCallable::t_CaptureCallback`.
+typedef void (*t_CallableEntry)(PTuple, PTuple, PTuple); // TODO(rokuzmin): Move to `QirCallable::t_CallableEntry`.
+typedef void (*t_CaptureCallback)(PTuple, int32_t);      // TODO(rokuzmin): Move to `QirCallable::t_CaptureCallback`.
 struct QIR_SHARED_API QirCallable
 {
     static int constexpr Adjoint    = 1;
-    static int constexpr Controlled = 1 << 1;
+    static int constexpr Controlled = 1u << 1;
 
   private:
     static int constexpr TableSize = 4;