TMP / Fixing clang-tidy warnings ep.1

desktop/graphics/glfw_mgmt.cpp

@@ -13,6 +13,7 @@ using namespace progressia::main::logging;
 
 namespace progressia::desktop {
 
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables): global variable required for GLFW callbacks
 static GLFWwindow *window = nullptr;
 
 static void onGlfwError(int errorCode, const char *description);
@@ -42,7 +43,9 @@ void initializeGlfw() {
         title = accumulator.str();
     }
 
-    window = glfwCreateWindow(800, 800, title.c_str(), nullptr, nullptr);
+    constexpr auto windowDimensions = 800;
+    window = glfwCreateWindow(windowDimensions, windowDimensions, title.c_str(),
+                              nullptr, nullptr);
 
     glfwSetWindowSizeCallback(window, onWindowGeometryChange);
 

desktop/graphics/vulkan_common.cpp

@@ -26,7 +26,7 @@ Vulkan::Vulkan(std::vector<const char *> instanceExtensions,
                std::vector<const char *> validationLayers)
     :
 
-      frames(MAX_FRAMES_IN_FLIGHT), isRenderingFrame(false),
+      frames(MAX_FRAMES_IN_FLIGHT), currentFrame(0), isRenderingFrame(false),
       lastStartedFrame(0) {
 
     /*
@@ -57,7 +57,7 @@ Vulkan::Vulkan(std::vector<const char *> instanceExtensions,
 
         // Enable extensions
         {
-            uint32_t extensionCount;
+            uint32_t extensionCount = 0;
             vkEnumerateInstanceExtensionProperties(nullptr, &extensionCount,
                                                    nullptr);
             std::vector<VkExtensionProperties> available(extensionCount);
@@ -88,7 +88,7 @@ Vulkan::Vulkan(std::vector<const char *> instanceExtensions,
 
         // Enable validation layers
         {
-            uint32_t layerCount;
+            uint32_t layerCount = 0;
             vkEnumerateInstanceLayerProperties(&layerCount, nullptr);
             std::vector<VkLayerProperties> available(layerCount);
             vkEnumerateInstanceLayerProperties(&layerCount, available.data());
@@ -153,8 +153,9 @@ Vulkan::Vulkan(std::vector<const char *> instanceExtensions,
         vkEnumeratePhysicalDevices(instance, &deviceCount, vkDevices.data());
 
         std::vector<PhysicalDevice> choices;
+        choices.reserve(deviceCount);
         for (const auto &vkDevice : vkDevices) {
-            choices.push_back(PhysicalDevice(vkDevice));
+            choices.emplace_back(PhysicalDevice(vkDevice));
         }
 
         const auto &result =
@@ -251,7 +252,6 @@ Vulkan::Vulkan(std::vector<const char *> instanceExtensions,
     for (auto &container : frames) {
         container.emplace(*this);
     }
-    currentFrame = 0;
 
     gint = std::make_unique<progressia::main::GraphicsInterface>(this);
 }
@@ -378,9 +378,9 @@ Frame *Vulkan::getCurrentFrame() {
     return nullptr;
 }
 
-uint64_t Vulkan::getLastStartedFrame() { return lastStartedFrame; }
+uint64_t Vulkan::getLastStartedFrame() const { return lastStartedFrame; }
 
-std::size_t Vulkan::getFrameInFlightIndex() { return currentFrame; }
+std::size_t Vulkan::getFrameInFlightIndex() const { return currentFrame; }
 
 bool Vulkan::startRender() {
     if (currentFrame >= MAX_FRAMES_IN_FLIGHT - 1) {
@@ -416,16 +416,21 @@ void Vulkan::waitIdle() {
  * VulkanErrorHandler
  */
 
-VulkanErrorHandler::VulkanErrorHandler(Vulkan &vulkan) : vulkan(vulkan) {
+VulkanErrorHandler::VulkanErrorHandler(Vulkan &vulkan)
+    : debugMessenger(nullptr), vulkan(vulkan) {
     // do nothing
 }
 
-VulkanErrorHandler::~VulkanErrorHandler() {
 #ifdef VULKAN_ERROR_CHECKING
+VulkanErrorHandler::~VulkanErrorHandler() {
+    if (debugMessenger != nullptr) {
         vulkan.callVoid("vkDestroyDebugUtilsMessengerEXT",
                         (VkDebugUtilsMessengerEXT)debugMessenger, nullptr);
-#endif
     }
+}
+#else
+VulkanErrorHandler::~VulkanErrorHandler() = default;
+#endif
 
 #ifdef VULKAN_ERROR_CHECKING
 namespace {
@@ -440,7 +445,8 @@ debugCallback(VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity,
         return VK_FALSE;
     }
 
-    [[maybe_unused]] auto &vk = *reinterpret_cast<const Vulkan *>(pUserData);
+    [[maybe_unused]] const auto &vk =
+        *reinterpret_cast<const Vulkan *>(pUserData);
 
     const char *severityStr =
         messageSeverity >= VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT
@@ -451,7 +457,7 @@ debugCallback(VkDebugUtilsMessageSeverityFlagBitsEXT messageSeverity,
             ? "info"
             : "verbose";
 
-    const char *typeStr;
+    const char *typeStr = "";
     switch (messageType) {
     case VK_DEBUG_UTILS_MESSAGE_TYPE_GENERAL_BIT_EXT:
         typeStr = "general";
@@ -508,8 +514,9 @@ VulkanErrorHandler::attachDebugProbe(VkInstanceCreateInfo &createInfo) {
 
 #else
 
-    (void)createInfo;
+    (void)createInfo; // unused argument
-    return std::unique_ptr<VkDebugUtilsMessengerCreateInfoEXT>();
+    (void)this;       // not static
+    return {};
 
 #endif
 }
@@ -528,6 +535,7 @@ void VulkanErrorHandler::onInstanceReady() {
 #endif
 }
 
+// NOLINTNEXTLINE(readability-convert-member-functions-to-static): future-proofing
 void VulkanErrorHandler::handleVkResult(const char *errorMessage,
                                         VkResult result) {
     if (result == VK_SUCCESS) {
@@ -543,7 +551,7 @@ void VulkanErrorHandler::handleVkResult(const char *errorMessage,
  * Surface
  */
 
-Surface::Surface(Vulkan &vulkan) : vulkan(vulkan) {
+Surface::Surface(Vulkan &vulkan) : vk(), vulkan(vulkan) {
     vulkan.handleVkResult("Could not create window surface (what?)",
                           glfwCreateWindowSurface(vulkan.getInstance(),
                                                   getGLFWWindowHandle(),
@@ -558,7 +566,7 @@ VkSurfaceKHR Surface::getVk() { return vk; }
  * Queue
  */
 
-Queue::Queue(Test test) : test(test) {
+Queue::Queue(Test test) : test(std::move(test)), vk() {
     // do nothing
 }
 
@@ -614,7 +622,7 @@ Queues::Queues(VkPhysicalDevice physicalDevice, Vulkan &vulkan)
 
     for (std::size_t index = 0; index < queueFamilyCount; index++) {
 
-        for (auto queue : {&graphicsQueue, &presentQueue}) {
+        for (auto *queue : {&graphicsQueue, &presentQueue}) {
             if (!queue->isSuitable(physicalDevice, index, vulkan,
                                    properties[index])) {
                 continue;
@@ -629,12 +637,10 @@ Queues::Queues(VkPhysicalDevice physicalDevice, Vulkan &vulkan)
     }
 }
 
-Queues::~Queues() {
+Queues::~Queues() = default;
-    // do nothing
-}
 
 void Queues::storeHandles(VkDevice device) {
-    for (auto queue : {&graphicsQueue, &presentQueue}) {
+    for (auto *queue : {&graphicsQueue, &presentQueue}) {
         vkGetDeviceQueue(device, queue->getFamilyIndex(), 0, &queue->vk);
     }
 }
@@ -643,7 +649,7 @@ std::unique_ptr<Queues::CreationRequest>
 Queues::requestCreation(VkDeviceCreateInfo &createInfo) const {
 
     std::unique_ptr result = std::make_unique<CreationRequest>();
-    result->priority = 1.0f;
+    result->priority = 1.0F;
 
     std::unordered_set<uint32_t> uniqueQueues;
     for (const auto *queue : {&graphicsQueue, &presentQueue}) {
@@ -668,7 +674,7 @@ Queues::requestCreation(VkDeviceCreateInfo &createInfo) const {
 }
 
 bool Queues::isComplete() const {
-    for (auto queue : {&graphicsQueue, &presentQueue}) {
+    for (const auto *queue : {&graphicsQueue, &presentQueue}) {
         if (!queue->familyIndex.has_value()) {
             return false;
         }
@@ -686,7 +692,7 @@ const Queue &Queues::getPresentQueue() const { return presentQueue; }
  */
 
 CommandPool::CommandPool(Vulkan &vulkan, const Queue &queue)
-    : queue(queue), vulkan(vulkan) {
+    : pool(), queue(queue), vulkan(vulkan) {
 
     VkCommandPoolCreateInfo poolInfo{};
     poolInfo.sType = VK_STRUCTURE_TYPE_COMMAND_POOL_CREATE_INFO;
@@ -709,12 +715,13 @@ VkCommandBuffer CommandPool::allocateCommandBuffer() {
     allocInfo.commandPool = pool;
     allocInfo.commandBufferCount = 1;
 
-    VkCommandBuffer commandBuffer;
+    auto *commandBuffer = VkCommandBuffer();
     vkAllocateCommandBuffers(vulkan.getDevice(), &allocInfo, &commandBuffer);
 
     return commandBuffer;
 }
 
+// NOLINTNEXTLINE(readability-convert-member-functions-to-static): future-proofing
 void CommandPool::beginCommandBuffer(VkCommandBuffer commandBuffer,
                                      VkCommandBufferUsageFlags usage) {
     VkCommandBufferBeginInfo beginInfo{};

desktop/graphics/vulkan_common.h

@@ -135,8 +135,8 @@ class Vulkan : public VkObjectWrapper {
     bool startRender();
     void endRender();
 
-    uint64_t getLastStartedFrame();
+    uint64_t getLastStartedFrame() const;
-    std::size_t getFrameInFlightIndex();
+    std::size_t getFrameInFlightIndex() const;
 
     void waitIdle();
 
@@ -192,12 +192,13 @@ class VulkanErrorHandler : public VkObjectWrapper {
     Vulkan &vulkan;
 
   public:
-    VulkanErrorHandler(Vulkan &);
+    VulkanErrorHandler(Vulkan &vulkan);
 
     std::unique_ptr<VkDebugUtilsMessengerCreateInfoEXT>
     attachDebugProbe(VkInstanceCreateInfo &);
     void onInstanceReady();
 
+    // NOLINTNEXTLINE(performance-trivially-destructible): fixing this makes code less readable due to use of macros in implementation
     ~VulkanErrorHandler();
 
     void handleVkResult(const char *errorMessage, VkResult result);
@@ -209,7 +210,7 @@ class Surface : public VkObjectWrapper {
     Vulkan &vulkan;
 
   public:
-    Surface(Vulkan &);
+    Surface(Vulkan &vulkan);
     ~Surface();
 
     VkSurfaceKHR getVk();
@@ -226,7 +227,7 @@ class Queue {
 
     friend class Queues;
 
-    Queue(Test);
+    Queue(Test test);
 
   public:
     bool isSuitable(VkPhysicalDevice, uint32_t familyIndex, Vulkan &,
@@ -275,7 +276,7 @@ class CommandPool : public VkObjectWrapper {
                             VkCommandBufferUsageFlags usage);
 
   public:
-    CommandPool(Vulkan &, const Queue &);
+    CommandPool(Vulkan &vulkan, const Queue &queue);
     ~CommandPool();
 
     VkCommandBuffer beginSingleUse();

desktop/main.cpp

@@ -13,7 +13,8 @@ int main(int argc, char *argv[]) {
     using namespace progressia;
 
     for (int i = 1; i < argc; i++) {
-        char *arg = argv[i];
+        char *arg = argv
+            [i]; // NOLINT(cppcoreguidelines-pro-bounds-pointer-arithmetic): infeasible to avoid in C++17
         if (strcmp(arg, "--version") == 0 || strcmp(arg, "-v") == 0) {
             std::cout << main::meta::NAME << " " << main::meta::VERSION << "+"
                       << main::meta::BUILD_ID << " (version number "

tools/clang-tidy/clang-tidy.yml

@@ -4,4 +4,38 @@ Checks: "-*,\
   modernize-*,\
   performance-*,\
   readability-*,\
-  clang-diagnostic-*"
+  clang-diagnostic-*,\
+  -modernize-use-trailing-return-type,\
+  -readability-implicit-bool-conversion,\
+  -cppcoreguidelines-pro-bounds-array-to-pointer-decay,\
+  -cppcoreguidelines-pro-type-reinterpret-cast,\
+  -cppcoreguidelines-pro-bounds-constant-array-index,\
+  -readability-else-after-return,\
+  -readability-named-parameter,\
+  -readability-use-anyofallof"
+
+# modernize-use-trailing-return-type
+#   ignore reason: reduces readability
+
+# readability-implicit-bool-conversion
+#   ignore reason: expected use by C libraries (GLFW, Vulkan API)
+
+# cppcoreguidelines-pro-bounds-array-to-pointer-decay
+#   ignore reason: expected use by C libraries
+
+# cppcoreguidelines-pro-type-reinterpret-cast
+#   ignore reason: expected use by C libraries
+
+# cppcoreguidelines-pro-bounds-constant-array-index
+#   ignore reason: infeasible to avoid without GSL
+
+# readability-else-after-return
+#   ignore reason: personal preference of OLEGSHA (using 'else' helps highlight
+#   branches in code)
+
+# readability-named-parameter
+#   ignore reason: using GCC convention is clear enough
+
+# readability-use-anyofallof
+#   ignore reason: these relatively obscure functions reduce readability by
+#   increasing code complexity and verbosity