Fix most warnings (#35)

* Turn on warnings

* -Wno-unused-parameter -Wno-unused-variable -Wno-unused-function

* Fix -Wformat

```
warning: format specifies type 'unsigned long long' but the argument has type 'uint64_t' (aka 'unsigned long') [-Wformat]
```

* Fix -Wrange-loop-construct warning

```
warning: loop variable 'it' creates a copy from type 'std::pair<const std::basic_string<char>, RT64::PresetLights::Light> const' [-Wrange-loop-construct]
```

* -Wsign-compare

```
comparison of integers of different signs: 'int32_t' (aka 'int') and 'uint32_t' (aka 'unsigned int')
```

* -Wrange-loop-construct

```
warning: loop variable ‘it’ creates a copy from type ‘const std::pair<const std::__cxx11::basic_string<char>, RT64::PresetDrawCall>’
```

* rerun

* Fix -Wparentheses

```
warning: suggest parentheses around arithmetic in operand of ‘^’
```

* -Wsign-compare

```
warning: comparison of integer expressions of different signedness: ‘int32_t’ {aka ‘int’} and ‘unsigned int’
```

* Update hlslpp to fix a number of warnings

* Remove `#pragma once` in cpp file

```
warning: #pragma once in main file
```

* -Wswitch

```
warning: enumeration value ‘None’ not handled in switch
```

* -Wformat-security

```
warning: format not a string literal and no format arguments
```

* regen

* Move warnings to clang check

* Fix -Wdelete-non-abstract-non-virtual-dtor

```
warning: destructor called on non-final 'RT64::PresetMaterial' that has virtual functions but non-virtual destructor
```

* remove warnings file

* Fix one unused parameter

* Fix assert wrapping

* Add -Werror flag

* Change type of `dstIndex` to `uint32_t`

* More uses I missed

* Fix new warnings

.github/workflows/validate.yml

@@ -34,7 +34,7 @@ jobs:
         run: |
           sudo apt-get update
           sudo apt-get install -y ninja-build libsdl2-dev libgtk-3-dev
-          
+
           # Install SDL2
           echo ::group::install SDL2
 
@@ -57,8 +57,8 @@ jobs:
         run: |-
           # enable ccache
           export PATH="/usr/lib/ccache:/usr/local/opt/ccache/libexec:$PATH"
-          
-          cmake -DCMAKE_BUILD_TYPE=${{ matrix.type }} -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_MAKE_PROGRAM=ninja -G Ninja -S . -B cmake-build
+
+          cmake -DCMAKE_BUILD_TYPE=${{ matrix.type }} -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_MAKE_PROGRAM=ninja -G Ninja -S . -B cmake-build
           cmake --build cmake-build --config ${{ matrix.type }} --target rt64 -j $(nproc)
       - name: Build RT64 (Windows)
         if: runner.os == 'Windows'
@@ -66,6 +66,6 @@ jobs:
           # enable ccache
           set $env:PATH="$env:USERPROFILE/.cargo/bin;$env:PATH"
           $cpuCores = (Get-CimInstance -ClassName Win32_Processor).NumberOfLogicalProcessors
-          
+
           cmake -DCMAKE_BUILD_TYPE=${{ matrix.type }} -DCMAKE_CXX_COMPILER_LAUNCHER=ccache -DCMAKE_C_COMPILER_LAUNCHER=ccache -DCMAKE_MAKE_PROGRAM=ninja -G Ninja -S . -B cmake-build
           cmake --build cmake-build --config ${{ matrix.type }} --target rt64 -j $cpuCores

CMakeLists.txt

@@ -57,7 +57,9 @@ add_executable(file_to_c src/tools/file_to_c.cpp)
 add_subdirectory(src/contrib/nativefiledialog-extended)
 
 if (CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
-    add_compile_options(-Wno-nullability-completeness)
+    add_compile_options(-Wall -Wextra)
+    add_compile_options(-Wno-nullability-completeness -Wno-unused-parameter -Wno-unused-variable -Wno-unused-function -Wno-unused-but-set-variable -Wno-missing-field-initializers)
+    add_compile_options(-Werror)
 endif()
 
 set(ANDROID_PLATFORM android-24)

src/common/rt64_thread.cpp

@@ -60,6 +60,7 @@ namespace RT64 {
         // that are available without root privileges are lower priority. Instead you can set the thread's "nice" value, which ranges
         // from -20 to 19 (lower being higher priority). However, by strict POSIX spec "nice" is meant to be per-process instead of
         // per-thread. Therefore to avoid issues in case Linux is modified to match the spec in the future, this function does nothing.
+        (void)priority;
 #   else
         static_assert(false, "Unimplemented");
 #   endif

src/contrib/hlslpp

@@ -1 +1 @@
-Subproject commit 240d7b5d5564d359ec79dfb27b197469273e4256
+Subproject commit 9e664ef8b61b5ee457976f56a27ac4a92cd81d0e

src/gbi/rt64_gbi.cpp

@@ -5,6 +5,7 @@
 #include "rt64_gbi.h"
 
 #include <cassert>
+#include <cinttypes>
 
 #include "rt64_gbi_extended.h"
 #include "rt64_gbi_f3d.h"
@@ -171,7 +172,7 @@ namespace RT64 {
 
             // If we used an unknown value in the database, print the hash.
             if (gbiSegment.hashValue == GBI_UNKNOWN_HASH_NUMB) {
-                fprintf(stdout, "HASH: 0x%016llXULL SIZE: 0x%X\n", rdramHash, rdramHashed);
+                fprintf(stdout, "HASH: 0x%016" PRIX64 "ULL SIZE: 0x%X\n", rdramHash, rdramHashed);
             }
         };
 
@@ -290,8 +291,8 @@ namespace RT64 {
                 const uint8_t *twoPointResult = std::search(searchResult, searchResultEnd, twoPointPattern, twoPointPattern + std::size(twoPointPattern) - 1);
                 if ((f3dexResult != searchResultEnd) && (twoPointResult != searchResultEnd)) {
                     const uint32_t lastOverlay = *reinterpret_cast<const uint32_t *>(&RDRAM[dataAddress + 0x410]);
-                    const uint16_t lastOverlaySize = *reinterpret_cast<const uint16_t *>(&RDRAM[dataAddress + 0x414 ^ 2]);
-                    const uint16_t estimatedDataSize = *reinterpret_cast<const uint16_t *>(&RDRAM[dataAddress + 0x380 ^ 2]);
+                    const uint16_t lastOverlaySize = *reinterpret_cast<const uint16_t *>(&RDRAM[(dataAddress + 0x414) ^ 2]);
+                    const uint16_t estimatedDataSize = *reinterpret_cast<const uint16_t *>(&RDRAM[(dataAddress + 0x380) ^ 2]);
                     const uint32_t textSize = lastOverlay + lastOverlaySize + 1;
                     fprintf(stderr, "Detected text size is 0x%X\n", textSize);
                     fprintf(stderr, "Estimated data size is 0x%X\n", estimatedDataSize);
@@ -300,7 +301,7 @@ namespace RT64 {
                 const uint8_t *s2dexResult = std::search(searchResult, searchResultEnd, s2dexPattern, s2dexPattern + std::size(s2dexPattern) - 1);
                 if ((s2dexResult != searchResultEnd) && (twoPointResult != searchResultEnd)) {
                     const uint32_t lastOverlay = *reinterpret_cast<const uint32_t *>(&RDRAM[dataAddress + 0x31C]);
-                    const uint16_t lastOverlaySize = *reinterpret_cast<const uint16_t *>(&RDRAM[dataAddress + 0x320 ^ 2]);
+                    const uint16_t lastOverlaySize = *reinterpret_cast<const uint16_t *>(&RDRAM[(dataAddress + 0x320) ^ 2]);
                     const uint32_t textSize = lastOverlay + lastOverlaySize + 1;
                     fprintf(stderr, "Detected text size is 0x%X\n", textSize);
                 }

src/gui/rt64_debugger_inspector.cpp

@@ -261,7 +261,7 @@ namespace RT64 {
         uint32_t totalSyncCount = 0;
         for (uint32_t f = 0; f < workload.fbPairCount; f++) {
             auto &fbPair = workload.fbPairs[f];
-            if (renderer.framebufferIndex == f) {
+            if ((uint32_t)renderer.framebufferIndex == f) {
                 renderer.framebufferAddress = renderer.framebufferDepth ? fbPair.depthImage.address : fbPair.colorImage.address;
             }
 
@@ -896,7 +896,7 @@ namespace RT64 {
                                     ImGui::PushID(l);
                                     
                                     if (openCall) {
-                                        bool open = (openLoadIndex == (callDesc.loadIndex + l));
+                                        bool open = ((uint32_t)openLoadIndex == (callDesc.loadIndex + l));
                                         ImGui::SetNextItemOpen(open);
                                         if (open) {
                                             ImGui::SetScrollHereY(0.0f);
@@ -984,7 +984,7 @@ namespace RT64 {
                                     const DrawCallTile &callTile = drawData.callTiles[callDesc.tileIndex + t];
                                     const LoadTile &loadTile = callTile.loadTile;
                                     if (openCall) {
-                                        bool open = (openTileIndex == (callDesc.tileIndex + t));
+                                        bool open = ((uint32_t)openTileIndex == (callDesc.tileIndex + t));
                                         ImGui::SetNextItemOpen(open);
                                         if (open) {
                                             ImGui::SetScrollHereY(0.0f);

src/hle/rt64_light_manager.cpp

@@ -109,7 +109,7 @@ namespace RT64 {
         hlslpp::float3 sunCol = { 0.8f, 0.7f, 0.6f };
         float biggestIntensity = 0;
         int biggestDirLight = -1;
-        for (int i = 0; i < directionalLights.size(); i++) {
+        for (size_t i = 0; i < directionalLights.size(); i++) {
             if (directionalLights[i].intensityTotal > biggestIntensity) {
                 biggestDirLight = i;
                 biggestIntensity = directionalLights[i].intensityTotal;

src/hle/rt64_rsp.cpp

@@ -288,7 +288,7 @@ namespace RT64 {
         modelViewProjChanged = changed;
     }
 
-    void RSP::setVertex(uint32_t address, uint8_t vtxCount, uint8_t dstIndex) {
+    void RSP::setVertex(uint32_t address, uint8_t vtxCount, uint32_t dstIndex) {
         if ((dstIndex >= RSP_MAX_VERTICES) || ((dstIndex + vtxCount) > RSP_MAX_VERTICES)) {
             assert(false && "Vertex indices are not valid. DL is possibly corrupted.");
             return;
@@ -300,7 +300,7 @@ namespace RT64 {
         setVertexCommon<true>(dstIndex, dstIndex + vtxCount);
     }
     
-    void RSP::setVertexPD(uint32_t address, uint8_t vtxCount, uint8_t dstIndex) {
+    void RSP::setVertexPD(uint32_t address, uint8_t vtxCount, uint32_t dstIndex) {
         if ((dstIndex >= RSP_MAX_VERTICES) || ((dstIndex + vtxCount) > RSP_MAX_VERTICES)) {
             assert(false && "Vertex indices are not valid. DL is possibly corrupted.");
             return;
@@ -326,7 +326,7 @@ namespace RT64 {
         setVertexCommon<true>(dstIndex, dstIndex + vtxCount);
     }
 
-    void RSP::setVertexEXV1(uint32_t address, uint8_t vtxCount, uint8_t dstIndex) {
+    void RSP::setVertexEXV1(uint32_t address, uint8_t vtxCount, uint32_t dstIndex) {
         if ((dstIndex >= RSP_MAX_VERTICES) || ((dstIndex + vtxCount) > RSP_MAX_VERTICES)) {
             assert(false && "Vertex indices are not valid. DL is possibly corrupted.");
             return;
@@ -1099,7 +1099,7 @@ namespace RT64 {
             int &stackSize = proj ? extended.viewProjMatrixIdStackSize : extended.modelMatrixIdStackSize;
             bool &stackChanged = proj ? extended.viewProjMatrixIdStackChanged : extended.modelMatrixIdStackChanged;
             if (push) {
-                if (stackSize < stack.size()) {
+                if ((size_t)stackSize < stack.size()) {
                     stackSize++;
                 }
                 else {

src/hle/rt64_rsp.h

@@ -232,9 +232,9 @@ namespace RT64 {
         void computeModelViewProj();
         void specialComputeModelViewProj();
         void setModelViewProjChanged(bool changed);
-        void setVertex(uint32_t address, uint8_t vtxCount, uint8_t dstIndex);
-        void setVertexPD(uint32_t address, uint8_t vtxCount, uint8_t dstIndex);
-        void setVertexEXV1(uint32_t address, uint8_t vtxCount, uint8_t dstIndex);
+        void setVertex(uint32_t address, uint8_t vtxCount, uint32_t dstIndex);
+        void setVertexPD(uint32_t address, uint8_t vtxCount, uint32_t dstIndex);
+        void setVertexEXV1(uint32_t address, uint8_t vtxCount, uint32_t dstIndex);
         void setVertexColorPD(uint32_t address);
         template<bool addEmptyVelocity>
         void setVertexCommon(uint8_t dstIndex, uint8_t dstMax);

src/hle/rt64_state.cpp

@@ -2198,9 +2198,9 @@ namespace RT64 {
                     }
 #               endif
                     ImGui::NewLine();
-                    
-                    ImGui::Text("Offline Shaders: %d", ext.rasterShaderCache->offlineList.entries.size());
-                    ImGui::Text("Specialized Shaders: %d", ext.rasterShaderCache->shaderCount());
+
+                    ImGui::Text("Offline Shaders: %zu", ext.rasterShaderCache->offlineList.entries.size());
+                    ImGui::Text("Specialized Shaders: %u", ext.rasterShaderCache->shaderCount());
 
                     ImGui::NewLine();
 

src/hle/rt64_workload_queue.cpp

@@ -735,7 +735,7 @@ namespace RT64 {
                     }
 
                     // Do the resolve if using MSAA while target override is active and we're on the correct framebuffer pair index.
-                    if (usingMSAA && (overrideTarget != nullptr) && (overrideTargetFbPairIndex == f)) {
+                    if (usingMSAA && (overrideTarget != nullptr) && ((uint32_t)overrideTargetFbPairIndex == f)) {
                         overrideTarget->resize(ext.workloadGraphicsWorker, colorTarget->width, colorTarget->height);
                         overrideTarget->resolveFromTarget(ext.workloadGraphicsWorker, colorTarget);
                     }
@@ -831,7 +831,7 @@ namespace RT64 {
                 threadConfigurationUpdate(workloadConfig);
 
                 // FIXME: This is a very hacky way to find out if we need to advance the frame if the workload was paused for the first time.
-                if (!workload.paused || (!gameFrames[curFrameIndex].workloads.empty() && (gameFrames[curFrameIndex].workloads[0] != processCursor))) {
+                if (!workload.paused || (!gameFrames[curFrameIndex].workloads.empty() && (gameFrames[curFrameIndex].workloads[0] != (uint32_t)processCursor))) {
                     prevFrameIndex = curFrameIndex;
                     curFrameIndex = (curFrameIndex + 1) % gameFrames.size();
                 }

src/preset/rt64_preset.h

@@ -34,6 +34,7 @@ namespace RT64 {
     struct PresetBase {
         bool enabled = true;
 
+        virtual ~PresetBase() = default;
         virtual bool readJson(const json &jsonObj);
         virtual bool writeJson(json &jsonObj) const;
     };
@@ -60,7 +61,7 @@ namespace RT64 {
         }
 
         void writeJson(json &jroot) {
-            for (const auto it : presetMap) {
+            for (const auto& it : presetMap) {
                 json jpreset;
                 jpreset["name"] = it.first;
                 it.second.writeJson(jpreset);

src/preset/rt64_preset_light.cpp

@@ -79,7 +79,7 @@ namespace RT64 {
             return false;
         }
 
-        for (const auto it : lightMap) {
+        for (const auto& it : lightMap) {
             json jlight;
             jlight["name"] = it.first;
             jlight["description"] = it.second.description;

src/preset/rt64_preset_material.cpp

@@ -172,10 +172,10 @@ namespace RT64 {
             ImGui::SameLine();
 
             if (checkboxValue) {
-                ImGui::Text(name);
+                ImGui::Text("%s", name);
             }
             else {
-                ImGui::TextDisabled(name);
+                ImGui::TextDisabled("%s", name);
             }
 
             return checkboxValue;

src/render/rt64_framebuffer_renderer.cpp

@@ -1200,7 +1200,7 @@ namespace RT64 {
 
         bool depthState = false;
         worker->commandList->setFramebuffer(targetDrawCall.fbStorage->colorDepthWrite.get());
-        for (const auto pair : targetDrawCall.sceneIndices) {
+        for (const auto& pair : targetDrawCall.sceneIndices) {
 #       if RT_ENABLED
             if (pair.second) {
                 const auto &rtScene = targetDrawCall.rtScenes[pair.first];
@@ -1241,7 +1241,7 @@ namespace RT64 {
                 submitDepthAccess(worker, targetDrawCall.fbStorage, true, depthState);
                 submitRaytracingScene(worker, targetDrawCall.fbStorage->colorTarget, rtScene);
             }
-            else 
+            else
 #       endif
             {
                 const RasterScene &rasterScene = targetDrawCall.rasterScenes[pair.first];
@@ -1598,6 +1598,9 @@ namespace RT64 {
                             triangles.viewport = rawViewportWide;
                             break;
                         }
+                        case Projection::Type::None: {
+                            break;
+                        }
                         }
 
                         triangles.scissor = convertFixedRect(call.callDesc.scissorRect, p.resolutionScale, p.fbWidth, invRatioScale, extOriginPercentage, int32_t(horizontalMisalignment), call.callDesc.scissorLeftOrigin, call.callDesc.scissorRightOrigin);

src/render/rt64_native_target.cpp

@@ -271,7 +271,7 @@ namespace RT64 {
             writeShader = &shaderLibrary->fbWriteColor;
         }
         
-        assert((fmt != G_IM_FMT_DEPTH) || (siz == G_IM_SIZ_16b) && "Depth format is not allowed outside of 16-bits.");
+        assert(((fmt != G_IM_FMT_DEPTH) || (siz == G_IM_SIZ_16b)) && "Depth format is not allowed outside of 16-bits.");
         
         // We need at least one read buffer in the history to use to write the output to.
         if (readBufferHistory.empty()) {

src/render/rt64_texture_cache.cpp

@@ -197,7 +197,7 @@ namespace RT64 {
         else {
             const uint8_t *srcData = reinterpret_cast<const uint8_t *>(bytes);
             size_t offset = 0;
-            while ((offset + rowPitch) <= byteCount) {
+            while ((offset + rowPitch) <= (size_t)byteCount) {
                 memcpy(dstData, srcData, rowPitch);
                 srcData += rowPitch;
                 offset += rowPitch;

src/render/rt64_upscaler.cpp

@@ -2,8 +2,6 @@
 // RT64
 //
 
-#pragma once
-
 #include <cassert>
 
 #include "rt64_upscaler.h"