diff --git a/desktop/graphics/glfw_mgmt.cpp b/desktop/graphics/glfw_mgmt.cpp index 5f7e7f7..ff4279d 100644 --- a/desktop/graphics/glfw_mgmt.cpp +++ b/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); diff --git a/desktop/graphics/vulkan_common.cpp b/desktop/graphics/vulkan_common.cpp index 77a0962..1769c0e 100644 --- a/desktop/graphics/vulkan_common.cpp +++ b/desktop/graphics/vulkan_common.cpp @@ -26,7 +26,7 @@ Vulkan::Vulkan(std::vector instanceExtensions, std::vector 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 instanceExtensions, // Enable extensions { - uint32_t extensionCount; + uint32_t extensionCount = 0; vkEnumerateInstanceExtensionProperties(nullptr, &extensionCount, nullptr); std::vector available(extensionCount); @@ -88,7 +88,7 @@ Vulkan::Vulkan(std::vector instanceExtensions, // Enable validation layers { - uint32_t layerCount; + uint32_t layerCount = 0; vkEnumerateInstanceLayerProperties(&layerCount, nullptr); std::vector available(layerCount); vkEnumerateInstanceLayerProperties(&layerCount, available.data()); @@ -153,8 +153,9 @@ Vulkan::Vulkan(std::vector instanceExtensions, vkEnumeratePhysicalDevices(instance, &deviceCount, vkDevices.data()); std::vector 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 instanceExtensions, for (auto &container : frames) { container.emplace(*this); } - currentFrame = 0; gint = std::make_unique(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 - vulkan.callVoid("vkDestroyDebugUtilsMessengerEXT", - (VkDebugUtilsMessengerEXT)debugMessenger, nullptr); -#endif +VulkanErrorHandler::~VulkanErrorHandler() { + if (debugMessenger != nullptr) { + vulkan.callVoid("vkDestroyDebugUtilsMessengerEXT", + (VkDebugUtilsMessengerEXT)debugMessenger, nullptr); + } } +#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(pUserData); + [[maybe_unused]] const auto &vk = + *reinterpret_cast(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; - return std::unique_ptr(); + (void)createInfo; // unused argument + (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() { - // do nothing -} +Queues::~Queues() = default; 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::requestCreation(VkDeviceCreateInfo &createInfo) const { std::unique_ptr result = std::make_unique(); - result->priority = 1.0f; + result->priority = 1.0F; std::unordered_set 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{}; diff --git a/desktop/graphics/vulkan_common.h b/desktop/graphics/vulkan_common.h index 917c3e7..9ff1b7e 100644 --- a/desktop/graphics/vulkan_common.h +++ b/desktop/graphics/vulkan_common.h @@ -135,8 +135,8 @@ class Vulkan : public VkObjectWrapper { bool startRender(); void endRender(); - uint64_t getLastStartedFrame(); - std::size_t getFrameInFlightIndex(); + uint64_t getLastStartedFrame() const; + 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 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(); diff --git a/desktop/main.cpp b/desktop/main.cpp index 1ac6a22..7587a8a 100644 --- a/desktop/main.cpp +++ b/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 " diff --git a/tools/clang-tidy/clang-tidy.yml b/tools/clang-tidy/clang-tidy.yml index 1a68ba0..2ca29aa 100644 --- a/tools/clang-tidy/clang-tidy.yml +++ b/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