Adding Static Analysis and other checks to clang-tidy (#810)

* Stopped disabling default checks 'clang-diagnostic-*,clang-analyzer-*'.
* Added cert* check to clang-tidy
* Fixed the clang-format version for Linux.
* Fixed Env:/AGENT_OS

Author: kuzminrobin

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/.clang-tidy

@@ -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/QubitManager.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/arrays.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/callables.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/lib/QIR/strings.cpp

@@ -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/lib/QSharpCore/.clang-tidy

@@ -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