Static analysis guided fixes (#135)

* Add noexcept to box Add noexcept to rc Add noexcept to vector Add noexcept to string Add noexcept to delegate Misc. static analysis fixes * MSVC Core Guidelines analyze build step * Fix new json error in converter * Split up uuid library so static analysis is easier * Remove MSVC analyzer from CI The ecosystem just isn't there yet for this. - The annotations break compilation in other compilers. - MSVC still has too many bugs and false-positives in its GSL rules. - CMake and MSVC are difficult to configure together.
potatoengine · Sep 30, 2019 · 7f4d36d · 7f4d36d
1 parent 1603639
commit 7f4d36d
Show file tree

Hide file tree

Showing 57 changed files with 411 additions and 360 deletions.
diff --git a/source/bin_shell/private/shell_app.cpp b/source/bin_shell/private/shell_app.cpp
@@ -86,7 +86,7 @@ int up::ShellApp::initialize() {
         return 1;
     }
 
-#if UP_GPU_ENABLE_D3D11
+#if defined(UP_GPU_ENABLE_D3D11)
     if (_device == nullptr) {
         auto factory = CreateFactoryD3D11();
         _device = factory->createDevice(0);

diff --git a/source/bin_shell/private/shell_app.h b/source/bin_shell/private/shell_app.h
@@ -72,5 +72,5 @@ class up::ShellApp {
     box<CameraController> _cameraController;
     box<InputState> _inputState;
     float _lastFrameTime = 0.f;
-    std::chrono::nanoseconds _lastFrameDuration;
+    std::chrono::nanoseconds _lastFrameDuration = {};
 };
diff --git a/source/libassetdb/private/hash_cache.cpp b/source/libassetdb/private/hash_cache.cpp
@@ -12,7 +12,7 @@ auto up::HashCache::hashAssetContent(span<up::byte const> contents) noexcept ->
 
 auto up::HashCache::hashAssetStream(Stream& stream) -> up::uint64 {
     auto hasher = fnv1a();
-    up::byte buffer[32768];
+    up::byte buffer[8*1024];
     while (!stream.isEof()) {
         span<up::byte> read(buffer, sizeof(buffer));
         stream.read(read);

diff --git a/source/libecs/public/potato/ecs/query.h b/source/libecs/public/potato/ecs/query.h
@@ -40,8 +40,8 @@ namespace up {
         template <size_t... Indices>
         void _invoke(std::index_sequence<Indices...>, size_t, EntityId const*, view<void*>, Delegate& callback) const;
 
-        ComponentId _components[sizeof...(Components)];
-        ComponentId _sortedComponents[sizeof...(Components)];
+        ComponentId _components[sizeof...(Components)] = {};
+        ComponentId _sortedComponents[sizeof...(Components)] = {};
     };
 
     template <typename... Components>

diff --git a/source/librecon/private/converter_app.cpp b/source/librecon/private/converter_app.cpp
@@ -9,6 +9,7 @@
 #include "potato/runtime/filesystem.h"
 #include "potato/runtime/native.h"
 #include "potato/runtime/stream.h"
+#include "potato/runtime/json.h"
 #include "potato/assetdb/hash_cache.h"
 #include "converters/convert_hlsl.h"
 #include "converters/convert_copy.h"

diff --git a/source/librender/private/d3d11_backend/d3d11_buffer.cpp b/source/librender/private/d3d11_backend/d3d11_buffer.cpp
@@ -3,7 +3,7 @@
 #include "d3d11_buffer.h"
 #include "potato/spud/int_types.h"
 
-up::d3d11::BufferD3D11::BufferD3D11(GpuBufferType type, up::uint64 size, com_ptr<ID3D11Buffer> buffer)
+up::d3d11::BufferD3D11::BufferD3D11(GpuBufferType type, up::uint64 size, com_ptr<ID3D11Buffer> buffer) noexcept
     : _type(type),
       _size(size),
       _buffer(std::move(buffer)) {

diff --git a/source/librender/private/d3d11_backend/d3d11_buffer.h b/source/librender/private/d3d11_backend/d3d11_buffer.h
@@ -9,7 +9,7 @@
 namespace up::d3d11 {
     class BufferD3D11 final : public GpuBuffer {
     public:
-        BufferD3D11(GpuBufferType type, uint64 size, com_ptr<ID3D11Buffer> buffer);
+        BufferD3D11(GpuBufferType type, uint64 size, com_ptr<ID3D11Buffer> buffer) noexcept;
         ~BufferD3D11();
 
         GpuBufferType type() const noexcept override { return _type; }

diff --git a/source/librender/private/d3d11_backend/d3d11_factory.cpp b/source/librender/private/d3d11_backend/d3d11_factory.cpp
@@ -13,7 +13,7 @@ up::d3d11::FactoryD3D11::FactoryD3D11(com_ptr<IDXGIFactory2> dxgiFactory)
 
 up::d3d11::FactoryD3D11::~FactoryD3D11() = default;
 
-#if UP_GPU_ENABLE_D3D11
+#if defined(UP_GPU_ENABLE_D3D11)
 auto up::CreateFactoryD3D11() -> box<GpuDeviceFactory> {
     com_ptr<IDXGIFactory2> dxgiFactory;
     CreateDXGIFactory1(__uuidof(IDXGIFactory2), out_ptr(dxgiFactory));

diff --git a/source/libruntime/private/CMakeLists.txt b/source/libruntime/private/CMakeLists.txt
@@ -42,6 +42,14 @@ target_sources(potato_libruntime PRIVATE
     $<$<PLATFORM_ID:Windows>:${CMAKE_CURRENT_SOURCE_DIR}/thread_util.windows.cpp>
 )
 
+# UUID generation
+#
+target_sources(potato_libruntime PRIVATE
+    $<$<PLATFORM_ID:Darwin>:${CMAKE_CURRENT_SOURCE_DIR}/uuid.darwin.cpp>
+    $<$<PLATFORM_ID:Linux>:${CMAKE_CURRENT_SOURCE_DIR}/uuid.linux.cpp>
+    $<$<PLATFORM_ID:Windows>:${CMAKE_CURRENT_SOURCE_DIR}/uuid.windows.cpp>
+)
+
 # General runtime code
 #
 target_sources(potato_libruntime PRIVATE

diff --git a/source/libruntime/private/debug.cpp b/source/libruntime/private/debug.cpp
@@ -25,10 +25,10 @@ auto up::_detail::raiseFatalError(char const* file, int line, char const* failed
     std::array<uintptr, 64> addresses = {};
 
 #if !defined(NDEBUG)
-    std::array<callstack::TraceRecord, 64> records = {};
-    auto stack = callstack::readTrace(addresses);
+    auto records = std::array<callstack::TraceRecord, 20>{};
+    auto const stack = callstack::readTrace(addresses);
 
-    auto resolvedRecords = callstack::resolveTraceRecords(stack, records);
+    auto const resolvedRecords = callstack::resolveTraceRecords(stack, records);
     if (!resolvedRecords.empty()) {
         for (auto const& record : resolvedRecords) {
             format_into(buffer, "[{:016X}] ({}:{}) {}\r\n", record.address, record.filename.c_str(), record.line, record.symbol.c_str());
@@ -37,7 +37,7 @@ auto up::_detail::raiseFatalError(char const* file, int line, char const* failed
     else
 #endif // !defined(NDEBUG)
     {
-        for (uintptr addr : addresses) {
+        for (auto const addr : addresses) {
             format_into(buffer, "{:016X}\r\n", addr);
         }
     }

diff --git a/source/libruntime/private/debug.windows.cpp b/source/libruntime/private/debug.windows.cpp
@@ -15,7 +15,7 @@ namespace {
         char const* callstack = nullptr;
     };
 
-    void CopyToClipboard(DialogData const& data) {
+    void CopyToClipboard(DialogData const& data) noexcept {
         // copy into clipboard
         if (OpenClipboard(nullptr) == TRUE) {
             EmptyClipboard();
@@ -24,11 +24,13 @@ namespace {
             up::format_into(buffer, "ASSERTION FAILED: {}\r\n{}\r\n{}\r\n{}", data.condition, data.message, data.location, data.callstack);
 
             if (HANDLE handle = GlobalAlloc(GMEM_MOVEABLE, buffer.size() + 1)) {
-                void* data = GlobalLock(handle);
-                std::memcpy(data, buffer.data(), buffer.size());
-                GlobalUnlock(handle);
+                void* lockedData = GlobalLock(handle);
+                if (lockedData != nullptr) {
+                    std::memcpy(lockedData, buffer.data(), buffer.size());
+                    GlobalUnlock(handle);
 
-                SetClipboardData(CF_TEXT, handle);
+                    SetClipboardData(CF_TEXT, handle);
+                }
             }
             CloseClipboard();
         }

diff --git a/source/libruntime/private/json.cpp b/source/libruntime/private/json.cpp
@@ -8,7 +8,7 @@
 
 auto up::readJson(Stream& stream, nlohmann::json& json) -> IOResult {
     string text;
-    IOResult rs = readText(stream, text);
+    IOResult const rs = readText(stream, text);
     if (rs != IOResult::Success) {
         return rs;
     }
@@ -25,6 +25,10 @@ void up::to_json(nlohmann::json& json, string_view str) noexcept {
     json = std::string(str.data(), str.size());
 }
 
+void up::to_json(nlohmann::json& json, string const& str) noexcept {
+    json = std::string(str.data(), str.size());
+}
+
 void up::from_json(const nlohmann::json& json, string& str) noexcept {
     str.reset();
     if (json.is_string()) {

diff --git a/source/libruntime/private/logger.cpp b/source/libruntime/private/logger.cpp
@@ -6,8 +6,7 @@
 #    include "potato/runtime/win32_debug_receiver.h"
 #endif
 
-up::Logger::Logger(string name, LogSeverity minimumSeverity) noexcept : _name(std::move(name)), _minimumSeverity(minimumSeverity) {
-
+up::Logger::Logger(string name, LogSeverity minimumSeverity) : _name(std::move(name)), _minimumSeverity(minimumSeverity) {
     static auto standard = up::new_shared<up::StandardStreamReceiver>();
     attach(standard);
 

diff --git a/source/libruntime/private/native.cpp b/source/libruntime/private/native.cpp
@@ -6,7 +6,7 @@
 
 namespace up {
     namespace {
-        struct NativeInputBackend : public Stream::Backend {
+        struct NativeInputBackend final : public Stream::Backend {
             NativeInputBackend(std::ifstream stream) : _stream(std::move(stream)) {}
 
             bool isOpen() const noexcept override { return _stream.is_open(); }
@@ -55,7 +55,7 @@ namespace up {
             mutable std::ifstream _stream;
         };
 
-        struct NativeOutputBackend : public Stream::Backend {
+        struct NativeOutputBackend final : public Stream::Backend {
             NativeOutputBackend(std::ofstream stream) : _stream(std::move(stream)) {}
 
             bool isOpen() const noexcept override { return _stream.is_open(); }
@@ -67,10 +67,10 @@ namespace up {
             IOResult seek(SeekPosition position, Stream::difference_type offset) override {
                 return IOResult::UnsupportedOperation;
             }
-            Stream::difference_type tell() const override {
+            Stream::difference_type tell() const noexcept override {
                 return 0;
             }
-            Stream::difference_type remaining() const override {
+            Stream::difference_type remaining() const noexcept override {
                 return 0;
             }
 

diff --git a/source/libruntime/private/native.default.cpp b/source/libruntime/private/native.default.cpp
@@ -16,18 +16,20 @@ static auto errorCodeToResult(std::error_code ec) noexcept -> up::IOResult {
 }
 
 bool up::NativeFileSystem::fileExists(zstring_view path) const noexcept {
-    return std::filesystem::is_regular_file(std::string_view(path.c_str(), path.size()));
+    [[maybe_unused]] std::error_code ec;
+    return std::filesystem::is_regular_file(std::string_view(path.c_str(), path.size()), ec);
 }
 
 bool up::NativeFileSystem::directoryExists(zstring_view path) const noexcept {
-    return std::filesystem::is_directory(std::string_view(path.c_str(), path.size()));
+    [[maybe_unused]] std::error_code ec;
+    return std::filesystem::is_directory(std::string_view(path.c_str(), path.size()), ec);
 }
 
 auto up::NativeFileSystem::fileStat(zstring_view path, FileStat& outInfo) const -> IOResult {
     std::error_code ec;
     outInfo.size = std::filesystem::file_size(std::string_view(path.c_str(), path.size()), ec);
     outInfo.mtime = std::chrono::duration_cast<std::chrono::microseconds>(std::filesystem::last_write_time(std::string_view(path.c_str(), path.size()), ec).time_since_epoch()).count();
-    auto status = std::filesystem::status(std::string_view(path.c_str(), path.size()), ec);
+    auto const status = std::filesystem::status(std::string_view(path.c_str(), path.size()), ec);
     outInfo.type =
         status.type() == std::filesystem::file_type::regular ? FileType::Regular : status.type() == std::filesystem::file_type::directory ? FileType::Directory : status.type() == std::filesystem::file_type::symlink ? FileType::SymbolicLink : FileType::Other;
     return errorCodeToResult(ec);
@@ -88,8 +90,9 @@ auto up::NativeFileSystem::removeRecursive(zstring_view path) -> IOResult {
 }
 
 auto up::NativeFileSystem::currentWorkingDirectory() const noexcept -> string {
-    auto path = std::filesystem::current_path().generic_u8string();
-    return string(path.c_str(), path.size());
+    std::error_code ec;
+    auto path = std::filesystem::current_path(ec).generic_u8string();
+    return ec ? string(path.c_str(), path.size()) : string();
 }
 
 bool up::NativeFileSystem::currentWorkingDirectory(zstring_view path) {

diff --git a/source/libruntime/private/null.cpp b/source/libruntime/private/null.cpp
@@ -7,18 +7,18 @@ bool up::NullFileSystem::fileExists(zstring_view path) const noexcept { return f
 
 bool up::NullFileSystem::directoryExists(zstring_view path) const noexcept { return false; }
 
-auto up::NullFileSystem::fileStat(zstring_view path, FileStat& outInfo) const -> IOResult { return IOResult::UnsupportedOperation; }
+auto up::NullFileSystem::fileStat(zstring_view path, FileStat& outInfo) const noexcept -> IOResult { return IOResult::UnsupportedOperation; }
 
-auto up::NullFileSystem::openRead(zstring_view, FileOpenMode) const -> Stream { return {}; }
+auto up::NullFileSystem::openRead(zstring_view, FileOpenMode) const noexcept -> Stream { return {}; }
 
-auto up::NullFileSystem::openWrite(zstring_view, FileOpenMode) -> Stream { return {}; }
+auto up::NullFileSystem::openWrite(zstring_view, FileOpenMode) noexcept -> Stream { return {}; }
 
-auto up::NullFileSystem::enumerate(zstring_view, EnumerateCallback, EnumerateOptions) const -> EnumerateResult { return EnumerateResult::Continue; }
+auto up::NullFileSystem::enumerate(zstring_view, EnumerateCallback, EnumerateOptions) const noexcept -> EnumerateResult { return EnumerateResult::Continue; }
 
-auto up::NullFileSystem::createDirectories(zstring_view path) -> IOResult { return IOResult::UnsupportedOperation; }
+auto up::NullFileSystem::createDirectories(zstring_view path) noexcept -> IOResult { return IOResult::UnsupportedOperation; }
 
-auto up::NullFileSystem::copyFile(zstring_view from, zstring_view to) -> IOResult { return IOResult::UnsupportedOperation; }
+auto up::NullFileSystem::copyFile(zstring_view from, zstring_view to) noexcept -> IOResult { return IOResult::UnsupportedOperation; }
 
-auto up::NullFileSystem::remove(zstring_view path) -> IOResult { return IOResult::UnsupportedOperation; }
+auto up::NullFileSystem::remove(zstring_view path) noexcept -> IOResult { return IOResult::UnsupportedOperation; }
 
-auto up::NullFileSystem::removeRecursive(zstring_view path) -> IOResult { return IOResult::UnsupportedOperation; }
+auto up::NullFileSystem::removeRecursive(zstring_view path) noexcept -> IOResult { return IOResult::UnsupportedOperation; }