Fix build / sanitizer warnings

MikePopoloski · Jun 9, 2024 · 73b79be · 73b79be
1 parent 780c284
commit 73b79be
Show file tree

Hide file tree

Showing 5 changed files with 17 additions and 18 deletions.
diff --git a/CMakePresets.json b/CMakePresets.json
@@ -148,7 +148,7 @@
       "cacheVariables": {
         "CMAKE_BUILD_TYPE": "RelWithDebInfo",
         "CMAKE_CXX_COMPILER": "clang++-18",
-        "CMAKE_CXX_FLAGS": "-fsanitize=undefined,address -fno-sanitize-recover=undefined -fno-omit-frame-pointer -fno-common -Wno-deprecated-declarations",
+        "CMAKE_CXX_FLAGS": "-DSLANG_ASSERT_ENABLED -fsanitize=undefined,address -fno-sanitize-recover=undefined -fno-omit-frame-pointer -fno-common -Wno-deprecated-declarations",
         "CMAKE_EXE_LINKER_FLAGS": "-fsanitize=undefined,address -fno-sanitize-recover=undefined",
         "CMAKE_SHARED_LINKER_FLAGS": "-fsanitize=undefined,address -fno-sanitize-recover=undefined",
         "SLANG_CLANG_TIDY": "clang-tidy-18"

diff --git a/include/slang/ast/Compilation.h b/include/slang/ast/Compilation.h
@@ -432,10 +432,10 @@ class SLANG_EXPORT Compilation : public BumpAllocator {
     /// @{
 
     /// Registers a system subroutine handler, which can be accessed by compiled code.
-    void addSystemSubroutine(std::shared_ptr<SystemSubroutine> subroutine);
+    void addSystemSubroutine(const std::shared_ptr<SystemSubroutine>& subroutine);
 
     /// Registers a type-based system method handler, which can be accessed by compiled code.
-    void addSystemMethod(SymbolKind typeKind, std::shared_ptr<SystemSubroutine> method);
+    void addSystemMethod(SymbolKind typeKind, const std::shared_ptr<SystemSubroutine>& method);
 
     /// Gets a system subroutine with the given name, or nullptr if there is no such subroutine
     /// registered.

diff --git a/source/ast/Compilation.cpp b/source/ast/Compilation.cpp
@@ -1016,12 +1016,13 @@ void Compilation::addGateType(const PrimitiveSymbol& prim) {
     gateMap.emplace(prim.name, &prim);
 }
 
-void Compilation::addSystemSubroutine(std::shared_ptr<SystemSubroutine> subroutine) {
-    subroutineMap.emplace(subroutine->name, std::move(subroutine));
+void Compilation::addSystemSubroutine(const std::shared_ptr<SystemSubroutine>& subroutine) {
+    subroutineMap.emplace(subroutine->name, subroutine);
 }
 
-void Compilation::addSystemMethod(SymbolKind typeKind, std::shared_ptr<SystemSubroutine> method) {
-    methodMap.emplace(std::make_tuple(std::string_view(method->name), typeKind), std::move(method));
+void Compilation::addSystemMethod(SymbolKind typeKind,
+                                  const std::shared_ptr<SystemSubroutine>& method) {
+    methodMap.emplace(std::make_tuple(std::string_view(method->name), typeKind), method);
 }
 
 const SystemSubroutine* Compilation::getSystemSubroutine(std::string_view name) const {

diff --git a/source/ast/expressions/AssertionExpr.cpp b/source/ast/expressions/AssertionExpr.cpp
@@ -615,10 +615,8 @@ std::optional<SequenceRange> SimpleAssertionExpr::computeSequenceLengthImpl() co
 
     if (expr.kind == ExpressionKind::AssertionInstance) {
         if (auto& aie = expr.as<AssertionInstanceExpression>(); aie.type->isSequenceType()) {
-            if (auto aieSeqLength = aie.body.computeSequenceLength();
-                aieSeqLength.has_value() && (res < aieSeqLength.value())) {
-                return aieSeqLength.value();
-            }
+            if (auto aieSeqLength = aie.body.computeSequenceLength(); res < aieSeqLength)
+                return aieSeqLength;
         }
     }
     return res;
@@ -1178,14 +1176,8 @@ std::optional<SequenceRange> BinaryAssertionExpr::computeSequenceLengthImpl() co
         const auto rightLenVal = rightLen.value();
         return (rightLenVal < leftLenVal) ? leftLenVal : rightLenVal;
     }
-    else if (leftLen.has_value()) {
-        return leftLen.value();
-    }
-    else if (rightLen.has_value()) {
-        return rightLen.value();
-    }
 
-    return std::nullopt;
+    return leftLen.has_value() ? leftLen : rightLen;
 }
 
 void BinaryAssertionExpr::serializeTo(ASTSerializer& serializer) const {

diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt
@@ -54,6 +54,12 @@ if(CMAKE_SYSTEM_NAME MATCHES "Windows")
   target_sources(unittests PRIVATE ${PROJECT_SOURCE_DIR}/scripts/win32.manifest)
 endif()
 
+if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+  # Suppress annoying false positive Wstringop-overflow warnings
+  target_compile_options(unittests PUBLIC "-Wno-stringop-overflow")
+  target_link_options(unittests PUBLIC "-Wno-stringop-overflow")
+endif()
+
 # Copy the data directory for running tests from the build folder.
 add_custom_command(
   TARGET unittests