clang-tidy: Fix header regex and the resulting nags

.clang-tidy

@@ -26,8 +26,13 @@
 #    glm
 # cppcoreguidelines-pro-type-vararg:
 #    fprintf, I really don't like formatting with streams
+# cppcoreguidelines-rvalue-reference-param-not-moved
+#    false positives from WHEELS_MOV
+#    TODO: Is there a way to annotate the macro like custom asserts?
 # misc-no-recursion
 #    I do use recursion in places
+# misc-include-cleaner
+#    Not available on my laptop yet so let's skip it for now
 # modernize-use-trailing-return-type:
 #    Haven't touched professionally, best not add 'noise'
 # modernize-use-auto
@@ -63,6 +68,8 @@ Checks: "*,
         -cppcoreguidelines-pro-type-reinterpret-cast,
         -cppcoreguidelines-pro-type-union-access,
         -cppcoreguidelines-pro-type-vararg,
+        -cppcoreguidelines-rvalue-reference-param-not-moved,
+        -misc-include-cleaner,
         -misc-non-private-member-variables-in-classes,
         -misc-no-recursion,
         -modernize-use-trailing-return-type,
@@ -75,5 +82,5 @@ Checks: "*,
         -readability-uppercase-literal-suffix
 "
 WarningsAsErrors: '*'
-HeaderFilterRegex: '.*/prosper/include/.*'
+HeaderFilterRegex: '.*[/\\]+prosper[/\\]+src[/\\]+.*\.hpp'
 FormatStyle: file

src/Allocators.hpp

@@ -34,6 +34,9 @@ struct Allocators
     void destroy();
 };
 
+// This is depended on by Device and init()/destroy() order relative to other
+// similar globals is handled in main()
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 extern Allocators gAllocators;
 
 #endif // PROSPER_ALLOCATORS_HPP

src/Window.hpp

@@ -63,6 +63,9 @@ class Window
     bool m_resized{false};
 };
 
+// This is depended on by Device and init()/destroy() order relative to other
+// similar globals is handled in main()
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 extern Window gWindow;
 
 #endif // PROSPER_WINDOW_HPP

src/gfx/DescriptorAllocator.hpp

@@ -62,6 +62,9 @@ class DescriptorAllocator
 // This allocator should only be used for the descriptors that can live
 // until the end of the program. As such, reset() shouldn't be called so
 // that users can rely on the descriptors being there once allocated.
+// This is depended on by Device and init()/destroy() order relative to other
+// similar globals is handled in main()
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 extern DescriptorAllocator gStaticDescriptorsAlloc;
 
 #endif // PROSPER_GFX_DESCRIPTOR_ALLOCATOR_HPP

src/gfx/Device.hpp

@@ -205,6 +205,9 @@ class Device
     MemoryAllocationBytes m_memoryAllocations;
 };
 
+// This is depended on by Device and init()/destroy() order relative to other
+// similar globals is handled in main()
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 extern Device gDevice;
 
 #endif // PROSPER_GFX_DEVICE_HPP

src/gfx/Fwd.hpp

@@ -5,6 +5,7 @@
 class DescriptorAllocator;
 
 // Device.hpp
+// NOLINTNEXTLINE(bugprone-forward-declaration-namespace)
 class Device;
 struct DeviceProperties;
 struct MemoryAllocationBytes;

src/gfx/Resources.hpp

@@ -185,6 +185,7 @@ struct Buffer
     // NOTE: Remember to amend clone() new members are added
 
     Buffer() noexcept = default;
+    ~Buffer() = default;
     // Copying is probably a mistake so disable implicit copies
     Buffer(const Buffer &) = delete;
     Buffer(Buffer &&) noexcept = default;
@@ -193,7 +194,7 @@ struct Buffer
 
     // There are use cases for mirrored buffers in async loading so expose a
     // convenience clone
-    Buffer clone() const;
+    [[nodiscard]] Buffer clone() const;
 
     [[nodiscard]] wheels::Optional<vk::BufferMemoryBarrier2> transitionBarrier(
         BufferState newState, bool force_barrier = false);
@@ -235,6 +236,7 @@ struct TexelBuffer
     VmaAllocation allocation{nullptr};
 
     TexelBuffer() noexcept = default;
+    ~TexelBuffer() = default;
     // Copying is probably a mistake so disable implicit copies
     TexelBuffer(const TexelBuffer &) = delete;
     TexelBuffer(TexelBuffer &&) noexcept = default;
@@ -309,6 +311,7 @@ struct Image
     vk::DeviceSize rawByteSize{0};
 
     Image() noexcept = default;
+    ~Image() = default;
     // Copying is probably a mistake so disable implicit copies
     Image(const Image &) = delete;
     Image(Image &&) noexcept = default;

src/render/ComputePass.hpp

@@ -120,7 +120,7 @@ class ComputePass
     void destroyPipelines();
 
     void createDescriptorSets(
-        wheels::ScopedScratch scopeAlloc, const char *,
+        wheels::ScopedScratch scopeAlloc, const char *debugName,
         vk::ShaderStageFlags storageStageFlags);
 
     void createPipeline(

src/render/RenderResourceCollection.hpp

@@ -58,7 +58,7 @@ class RenderResourceCollection
 
   protected:
     void assertValidHandle(Handle handle) const;
-    wheels::StrSpan aliasedDebugName(Handle handle) const;
+    [[nodiscard]] wheels::StrSpan aliasedDebugName(Handle handle) const;
 
   private:
     static const uint64_t sNotInUseGenerationFlag = static_cast<size_t>(1)

src/render/RenderResources.hpp

@@ -68,6 +68,9 @@ class RenderResources
     bool m_initialized{false};
 };
 
+// This is depended on by Device and init()/destroy() order relative to other
+// similar globals is handled in main()
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 extern RenderResources gRenderResources;
 
 struct Transitions

src/render/Renderer.hpp

@@ -64,7 +64,7 @@ class Renderer
     void render(
         wheels::ScopedScratch scopeAlloc, vk::CommandBuffer cb,
         const Camera &cam, World &world, const vk::Rect2D &renderArea,
-        const SwapchainImage &swapImage, const uint32_t nextFrame,
+        const SwapchainImage &swapImage, uint32_t nextFrame,
         const Options &options);
 
   private:

src/render/SkyboxRenderer.hpp

@@ -23,14 +23,13 @@ class SkyboxRenderer
     SkyboxRenderer &operator=(SkyboxRenderer &&other) = delete;
 
     void init(
-        wheels::ScopedScratch scopeAlloc,
-        const vk::DescriptorSetLayout camDSLayout,
+        wheels::ScopedScratch scopeAlloc, vk::DescriptorSetLayout camDSLayout,
         const WorldDSLayouts &worldDSLayouts);
 
     void recompileShaders(
         wheels::ScopedScratch scopeAlloc,
         const wheels::HashSet<std::filesystem::path> &changedFiles,
-        const vk::DescriptorSetLayout camDSLayout,
+        vk::DescriptorSetLayout camDSLayout,
         const WorldDSLayouts &worldDSLayouts);
 
     struct RecordInOut
@@ -50,7 +49,7 @@ class SkyboxRenderer
     void destroyGraphicsPipelines();
 
     void createGraphicsPipelines(
-        const vk::DescriptorSetLayout camDSLayout,
+        vk::DescriptorSetLayout camDSLayout,
         const WorldDSLayouts &worldDSLayouts);
 
     bool m_initialized{false};

src/scene/Accessors.hpp

@@ -24,8 +24,8 @@ class TimeAccessor
     TimeAccessor(
         const float *data, uint32_t count, const Interval &interval) noexcept;
 
-    float endTimeS() const;
-    KeyFrameInterpolation interpolation(float timeS) const;
+    [[nodiscard]] float endTimeS() const;
+    [[nodiscard]] KeyFrameInterpolation interpolation(float timeS) const;
 
   private:
     const float *m_data{nullptr};
@@ -41,7 +41,7 @@ template <typename T> class ValueAccessor
     // Count is for vector elements, not individual float
     ValueAccessor(const uint8_t *data, uint32_t count) noexcept;
 
-    T read(uint32_t index) const;
+    [[nodiscard]] T read(uint32_t index) const;
 
   private:
     const uint8_t *m_data{nullptr};

src/scene/Animations.hpp

@@ -24,7 +24,7 @@ template <typename T> class Animation
         InterpolationType interpolation, TimeAccessor &&timeFrames,
         ValueAccessor<T> &&valueFrames);
 
-    float endTimeS() const;
+    [[nodiscard]] float endTimeS() const;
 
     void registerTarget(T &target);
     void update(float timeS);
@@ -69,7 +69,7 @@ template <typename T> void Animation<T>::update(float timeS)
 {
     const KeyFrameInterpolation interp = m_timeFrames.interpolation(timeS);
 
-    T firstValue;
+    T firstValue{};
     if (interp.t == 0.f && m_interpolation == InterpolationType::CubicSpline)
         // Three values per frame, property value is the middle one
         firstValue = m_valueFrames.read(interp.firstFrame * 3 + 1);

src/scene/Camera.hpp

@@ -121,7 +121,7 @@ class Camera
     // Applies an offset without touching the held one
     void applyOffset(const CameraOffset &offset);
 
-    FrustumCorners getFrustumCorners() const;
+    [[nodiscard]] FrustumCorners getFrustumCorners() const;
 
   private:
     void createBindingsReflection(wheels::ScopedScratch scopeAlloc);

src/scene/World.hpp

@@ -30,8 +30,8 @@ class World
     void endFrame();
 
     // Returns true if the visible scene was changed.
-    bool handleDeferredLoading(vk::CommandBuffer cb);
-    bool unbuiltBlases() const;
+    [[nodiscard]] bool handleDeferredLoading(vk::CommandBuffer cb);
+    [[nodiscard]] bool unbuiltBlases() const;
 
     void drawDeferredLoadingUi() const;
     // Returns true if the next frame will use a different scene
@@ -57,7 +57,7 @@ class World
     void updateBuffers(wheels::ScopedScratch scopeAlloc);
     // Has to be called after updateBuffers(). Returns true if new BLASes were
     // added.
-    bool buildAccelerationStructures(
+    [[nodiscard]] bool buildAccelerationStructures(
         wheels::ScopedScratch scopeAlloc, vk::CommandBuffer cb);
     void drawSkybox(vk::CommandBuffer cb) const;
 

src/scene/WorldData.cpp

@@ -185,9 +185,10 @@ vk::Filter getVkFilterMode(cgltf_int glEnum)
     case s_gl_linear_mipmap_nearest:
     case s_gl_linear_mimpap_linear:
         return vk::Filter::eLinear;
+    default:
+        LOG_ERR("Invalid gl filter %d", glEnum);
     }
 
-    LOG_ERR("Invalid gl filter %d", glEnum);
     return vk::Filter::eLinear;
 }
 
@@ -201,8 +202,10 @@ vk::SamplerAddressMode getVkAddressMode(cgltf_int glEnum)
         return vk::SamplerAddressMode::eMirroredRepeat;
     case s_gl_repeat:
         return vk::SamplerAddressMode::eRepeat;
+    default:
+        LOG_ERR("Invalid gl wrapping mode %d", glEnum);
     }
-    LOG_ERR("Invalid gl wrapping mode %d", glEnum);
+
     return vk::SamplerAddressMode::eClampToEdge;
 }
 

src/utils/Hashes.hpp

@@ -24,7 +24,9 @@ template <> struct Hash<std::filesystem::path>
     [[nodiscard]] uint64_t operator()(
         const std::filesystem::path &value) const noexcept
     {
-        return wyhash(value.string().c_str(), value.string().size(), 0, _wyp);
+        return wyhash(
+            value.string().c_str(), value.string().size(), 0,
+            (uint64_t const *)_wyp);
     }
 };
 

src/utils/InputHandler.hpp

@@ -88,6 +88,9 @@ class InputHandler
     wheels::Optional<MouseGesture> m_mouseGesture;
 };
 
+// This is depended on by Device and init()/destroy() order relative to other
+// similar globals is handled in main()
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 extern InputHandler gInputHandler;
 
 #endif // PROSPER_UTILS_INPUT_HANDLER_HPP

src/utils/Logger.cpp

@@ -31,9 +31,8 @@ namespace
 // This should be plenty for even the worst shader error messes and still small
 // enough to comfortably fit the 1MB stack on windows.
 const int sTmpStrLength = static_cast<int>(kilobytes(8));
-const char *const sOutOfSpaceError =
+const StaticArray sOutOfSpaceError =
     "\n[ ERROR: Logger ran out of tmp formatting space ]\n";
-const int sOutOfSpaceErrorLength = static_cast<int>(strlen(sOutOfSpaceError));
 
 // TODO:
 // InlineArray and resize_uninitialized() instead?
@@ -97,7 +96,7 @@ struct TmpStr
 
         // Include final null after the overflow error
         const int space = sTmpStrLength - asserted_cast<int>(writePtr) -
-                          sOutOfSpaceErrorLength - 1;
+                          asserted_cast<int>(sOutOfSpaceError.size()) - 1;
         WHEELS_ASSERT(space >= 0);
 
         const int charsWritten =
@@ -162,11 +161,11 @@ struct TmpStr
         {
             WHEELS_ASSERT(
                 sTmpStrLength - asserted_cast<int>(writePtr) >=
-                sOutOfSpaceErrorLength + 1);
+                asserted_cast<int>(sOutOfSpaceError.size()) + 1);
             // Copy the final null too
             memcpy(
-                str.data() + writePtr, sOutOfSpaceError,
-                sOutOfSpaceErrorLength + 1);
+                str.data() + writePtr, sOutOfSpaceError.data(),
+                asserted_cast<int>(sOutOfSpaceError.size()) + 1);
         }
     }
 };

src/utils/Profiler.hpp

@@ -284,6 +284,9 @@ class Profiler
         gAllocators.general};
 };
 
+// This is depended on by Device and init()/destroy() order relative to other
+// similar globals is handled in main()
+// NOLINTNEXTLINE(cppcoreguidelines-avoid-non-const-global-variables)
 extern Profiler gProfiler;
 
 // The scope variable is never accessed so let's reduce the noise with a macro

src/utils/Utils.hpp

@@ -23,19 +23,18 @@ template <typename T, typename V> constexpr T asserted_cast(V a)
 {
 #ifndef NDEBUG
     static_assert(
-        !std::is_floating_point<T>::value &&
+        !std::is_floating_point_v<T> &&
         "No assertions for floating point target type");
 
-    if constexpr (!std::is_same<T, V>::value && std::is_integral<T>::value)
+    if constexpr (!std::is_same_v<T, V> && std::is_integral_v<T>)
     {
         if (a >= 0)
         {
-            if constexpr (std::is_integral<V>::value)
+            if constexpr (std::is_integral_v<V>)
             {
                 if constexpr ((sizeof(T) < sizeof(V) ||
-                               (sizeof(T) == sizeof(V) &&
-                                std::is_signed<T>::value &&
-                                !std::is_signed<V>::value)))
+                               (sizeof(T) == sizeof(V) && std::is_signed_v<T> &&
+                                !std::is_signed_v<V>)))
                 {
                     if (a > static_cast<V>(std::numeric_limits<T>::max()))
                         WHEELS_ASSERT(!"overflow");
@@ -95,7 +94,7 @@ void writeRawSpan(std::ofstream &stream, wheels::Span<const T> span)
 
 inline void writeRawStrSpan(std::ofstream &stream, const wheels::StrSpan span)
 {
-    stream.write(span.data(), span.size());
+    stream.write(span.data(), asserted_cast<std::streamsize>(span.size()));
 }
 
 template <typename T> void writeRaw(std::ofstream &stream, const T &value)
@@ -151,6 +150,7 @@ inline void appendEnumVariantsAsDefines(
 // remainder. Assumes both inputs are positive. Returns 0 when dividend is 0.
 template <typename T>
     requires std::is_integral_v<T>
+// NOLINTNEXTLINE(bugprone-easily-swappable-parameters)
 T roundedUpQuotient(T dividend, T divisor)
 {
     if (dividend == 0) [[unlikely]]
@@ -174,9 +174,24 @@ struct DeferDummy
 template <class F> struct Deferrer
 {
     F f;
+    Deferrer(const F &f)
+    : f{f}
+    {
+    }
     ~Deferrer() { f(); }
+
+    Deferrer(const Deferrer<F> &) = delete;
+    Deferrer &operator=(const Deferrer<F> &) = delete;
+    Deferrer(Deferrer<F> &&) = delete;
+    Deferrer &operator=(Deferrer<F> &&) = delete;
 };
-template <class F> Deferrer<F> operator*(DeferDummy, F f) { return {f}; }
+template <class F> Deferrer<F> operator*(DeferDummy dummy, F f)
+{
+    (void)dummy;
+    return {f};
+}
+// Caller supplies the lambda scope, works as intended
+// NOLINTNEXTLINE(bugprone-macro-parentheses)
 #define defer auto TOKEN_APPEND(zzDefer, __LINE__) = DeferDummy{} *[&]()
 
 void setCurrentThreadName(const char *name);