From f2f42d08f9a75b859d2d6b6be415855c19d2e9cb Mon Sep 17 00:00:00 2001
From: Bryan Tan <bryantan@technius.net>
Date: Fri, 15 Dec 2023 02:39:37 -0800
Subject: [PATCH] Miscellaneous bugfixes (#23)

Summary:
This pull request includes several miscellaneous bugfixes, which (I think) should not introduce any breaking changes:

* Fix a gcc `-Wreturn-type` warning in `PowersetAbstractDomain` and `SmallSortedSetAbstractDomain` that causes build failures in downstream projects compiled with `-Werror -Wall`.
* Don't add test targets if the CMake variable `BUILD_TESTING` is falsey.
* Fix the exported CMake `sparta` target using the wrong directory as the include directory when installed

Pull Request resolved: https://github.com/facebook/SPARTA/pull/23

Reviewed By: arnaudvenet

Differential Revision: D52111351

Pulled By: arthaud

fbshipit-source-id: 97d9a89537ce77f575f0e073b43f5d81eeba6a08
---
 CMakeLists.txt                                | 20 +++++--------------
 include/sparta/PowersetAbstractDomain.h       |  5 +++++
 include/sparta/SmallSortedSetAbstractDomain.h |  4 ++++
 test/CMakeLists.txt                           | 17 ++++++++++++++++
 4 files changed, 31 insertions(+), 15 deletions(-)
 create mode 100644 test/CMakeLists.txt

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 2f3d3d4..f549e39 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -6,6 +6,7 @@
 cmake_minimum_required(VERSION 3.0.2)
 project("sparta")
 
+include(CTest)
 set(CMAKE_MODULE_PATH "${CMAKE_SOURCE_DIR}/cmake_modules" ${CMAKE_MODULE_PATH})
 include(Commons)
 
@@ -25,7 +26,7 @@ add_library(sparta INTERFACE)
 
 target_include_directories(sparta INTERFACE
   $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}/include>
-  $<INSTALL_INTERFACE:include>
+  $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}>
 )
 
 target_link_libraries(sparta INTERFACE ${Boost_LIBRARIES})
@@ -53,17 +54,6 @@ export(TARGETS sparta FILE sparta_target.cmake)
 ###################################################
 # test
 ###################################################
-file(GLOB test "test/*.cpp")
-
-include(CTest)
-# ${test} contains all paths to the test cpps
-foreach(testfile ${test})
-  # ${testfile} is in the format of test/SomeTest.cpp
-  string(REPLACE ".cpp" "_test" no_ext_name ${testfile})
-  # ${no_ext_name} is in the format of test/SomeTest_test
-  get_filename_component(test_bin ${no_ext_name} NAME)
-  # ${test_bin} is in the format of SomeTest_test
-  add_executable(${test_bin} ${testfile})
-  target_link_libraries(${test_bin} PRIVATE sparta gmock_main)
-  add_test(NAME ${testfile} COMMAND ${test_bin})
-endforeach()
+if (BUILD_TESTING)
+  add_subdirectory(test)
+endif()
diff --git a/include/sparta/PowersetAbstractDomain.h b/include/sparta/PowersetAbstractDomain.h
index 88e1ca5..6eef4e9 100644
--- a/include/sparta/PowersetAbstractDomain.h
+++ b/include/sparta/PowersetAbstractDomain.h
@@ -14,6 +14,7 @@
 #include <type_traits>
 
 #include <sparta/AbstractDomain.h>
+#include <sparta/Exceptions.h>
 
 namespace sparta {
 namespace pad_impl {
@@ -235,6 +236,10 @@ class PowersetAbstractDomain
       return this->get_value()->contains(e);
     }
     }
+    SPARTA_RUNTIME_CHECK(
+        false, internal_error() << error_msg("unknown AbstractValueKind"));
+    // Return false to suppress -Wreturn-type warning reported by gcc
+    return false;
   }
 
   friend std::ostream& operator<<(std::ostream& o, const Derived& s) {
diff --git a/include/sparta/SmallSortedSetAbstractDomain.h b/include/sparta/SmallSortedSetAbstractDomain.h
index 514f094..3d930bc 100644
--- a/include/sparta/SmallSortedSetAbstractDomain.h
+++ b/include/sparta/SmallSortedSetAbstractDomain.h
@@ -167,6 +167,10 @@ class SmallSortedSetAbstractDomain final
       return this->get_value()->contains(e);
     }
     }
+    SPARTA_RUNTIME_CHECK(
+        false, internal_error() << error_msg("unknown AbstractValueKind"));
+    // Return false to suppress -Wreturn-type warning reported by gcc
+    return false;
   }
 
   friend std::ostream& operator<<(std::ostream& out,
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
new file mode 100644
index 0000000..b36f911
--- /dev/null
+++ b/test/CMakeLists.txt
@@ -0,0 +1,17 @@
+# Copyright (c) Meta Platforms, Inc. and affiliates.
+#
+# This source code is licensed under the MIT license found in the
+# LICENSE file in the root directory of this source tree.
+
+file(GLOB test "*.cpp")
+
+foreach(testfile ${test})
+  # ${testfile} is in the format of SomeTest.cpp
+  string(REPLACE ".cpp" "_test" no_ext_name ${testfile})
+  # ${no_ext_name} is in the format of SomeTest_test
+  get_filename_component(test_bin ${no_ext_name} NAME)
+  # ${test_bin} is in the format of SomeTest_test
+  add_executable(${test_bin} ${testfile})
+  target_link_libraries(${test_bin} PRIVATE sparta gmock_main)
+  add_test(NAME ${testfile} COMMAND ${test_bin})
+endforeach()