build: Enable compiler warnings for all targets

Author: gf712

CMakeLists.txt

@@ -77,10 +77,26 @@ target_compile_options(project_warnings
   INTERFACE $<$<COMPILE_LANGUAGE:CXX>:-Wno-sign-conversion;-Wno-shadow;-Wno-implicit-fallthrough;-Wno-old-style-cast;-Wno-deprecated-copy;-Wno-missing-field-initializers;-Wno-null-dereference;-Wno-maybe-uninitialized;-Wno-stringop-overflow>
 )
 
+target_compile_options(project_options
+  # -fno-omit-frame-pointer is needed, otherwise we cannot access the stack base
+  # pointer reliably. The garbage collector scans the C++ stack conservatively
+  # for roots (see MarkSweepGC::collect_roots), so every target whose frames can
+  # be live across an allocation needs it.
+  INTERFACE $<$<COMPILE_LANGUAGE:CXX>:-fno-omit-frame-pointer>
+)
+
+# Every first-party target links against project_options and project_warnings
+# through this helper so they are all compiled the same way.
+include(PythonCppFlags)
+
+# check_cxx_source_compiles links the snippet, so it needs a main(); and
+# std::uint64_t needs <cstdint> rather than coming along with <bit>.
 check_cxx_source_compiles(
 	"#include <bit>
+  #include <cstdint>
   constexpr double f64v = 19880124.0;
-  constexpr auto u64v = std::bit_cast<std::uint64_t>(f64v);"
+  constexpr auto u64v = std::bit_cast<std::uint64_t>(f64v);
+  int main() { return u64v == 0; }"
 	STL_SUPPORTS_BIT_CAST)
 
 find_library(MATH_LIBRARY m)

CMakePresets.json

@@ -70,4 +70,4 @@
     }
   ]
 
-}
+}

cmake/PythonCppFlags.cmake

@@ -0,0 +1,36 @@
+# Helper for giving every first-party target the same compiler flags.
+#
+# The flags come from the external `project_options` package (added with CPM in
+# the top-level CMakeLists.txt), which exposes them as two INTERFACE targets:
+#   * project_options  - sanitizers, hardening, linker and optimisation flags
+#   * project_warnings - the warning set plus -Werror
+# The top-level CMakeLists.txt adjusts both to taste; everything else just links
+# against them through `python_cpp_link_project_options()` below. Third-party
+# code pulled in by CPM (spdlog, googletest, linenoise, ...) is deliberately
+# left alone.
+#
+# The helper exists because of the LLVM/MLIR target helpers
+# (add_mlir_library, add_mlir_conversion_library, add_mlir_translation_library,
+# ...): they compile their sources in a separate `obj.<name>` object library
+# rather than in `<name>` itself, and only forward include directories to it -
+# not the usage requirements of libraries linked afterwards. Linking the flags
+# to `<name>` alone would therefore silently compile nothing with them, so this
+# always covers the `obj.<name>` twin as well.
+
+include_guard(GLOBAL)
+
+function(python_cpp_link_project_options)
+  foreach(target ${ARGN})
+    foreach(name ${target} obj.${target})
+      if(NOT TARGET ${name})
+        continue()
+      endif()
+      get_target_property(type ${name} TYPE)
+      if(type STREQUAL "INTERFACE_LIBRARY")
+        target_link_libraries(${name} INTERFACE project_options project_warnings)
+      else()
+        target_link_libraries(${name} PRIVATE project_options project_warnings)
+      endif()
+    endforeach()
+  endforeach()
+endfunction()

integration/CMakeLists.txt

@@ -1,5 +1,6 @@
 add_executable(integration-tests_ program.cpp ../src/testing/main.cpp)
-target_link_libraries(integration-tests_ PRIVATE python-cpp gtest gtest_main cxxopts project_options project_warnings tsl::ordered_map)
+target_link_libraries(integration-tests_ PRIVATE python-cpp gtest gtest_main cxxopts tsl::ordered_map)
+python_cpp_link_project_options(integration-tests_)
 # gtest_discover_tests(integration-tests_)
 
 add_test(

src/CMakeLists.txt

@@ -269,8 +269,6 @@ add_executable(unittests_ ${UNITTEST_SOURCES})
 target_link_libraries(python-cpp
   PUBLIC spdlog m
   PRIVATE
-    project_options
-    project_warnings
     ICU::uc
     ICU::data
     ${GMPXX_LIBRARIES}
@@ -283,11 +281,7 @@ target_include_directories(python-cpp
   PRIVATE ${GMP_INCLUDES} ${CMAKE_CURRENT_BINARY_DIR}
 )
 
-target_compile_options(
-  python-cpp
-  PRIVATE
-  # -fno-omit-frame-pointer is needed, otherwise we cannot access the stack base pointer reliably
-  -fno-omit-frame-pointer)
+python_cpp_link_project_options(python-cpp)
 
 if(STL_SUPPORTS_BIT_CAST)
   target_compile_definitions(python-cpp PUBLIC "STL_SUPPORTS_BIT_CAST")
@@ -312,14 +306,6 @@ elseif(ENABLE_LLVM_BACKEND AND LLVM_FOUND)
   message(STATUS "Configuring LLVM backend")
   add_library(python-cpp-llvm ${LLVM_BACKEND_FILES})
 
-  target_compile_options(
-    python-cpp-llvm
-    PRIVATE -Wall
-            -Wextra
-            -Werror
-            -Wno-unused-parameter
-            -fno-omit-frame-pointer)
-
   add_library(llvm-interface INTERFACE)
   target_include_directories(llvm-interface INTERFACE . )
   # include llvm include directories as system paths to silence compiler warnings
@@ -337,7 +323,8 @@ elseif(ENABLE_LLVM_BACKEND AND LLVM_FOUND)
     orcjit
     x86asmparser
     x86codegen)
-  target_link_libraries(llvm-interface INTERFACE ${llvm_libs} project_options project_warnings)
+  target_link_libraries(llvm-interface INTERFACE ${llvm_libs})
+  python_cpp_link_project_options(llvm-interface)
   # TODO: not all versions of llvm are ready for C++20, figure out when to use this
   set_property(TARGET python-cpp-llvm PROPERTY CXX_STANDARD 17)
 
@@ -355,12 +342,14 @@ elseif(ENABLE_LLVM_BACKEND AND LLVM_FOUND)
   target_link_libraries(unittests_ PRIVATE python-cpp-llvm)
 endif()
 
-target_link_libraries(unittests_ PRIVATE python-cpp gtest gtest_main cxxopts project_options project_warnings tsl::ordered_map)
+target_link_libraries(unittests_ PRIVATE python-cpp gtest gtest_main cxxopts tsl::ordered_map)
 gtest_discover_tests(unittests_)
 
 add_executable(python repl/repl.cpp)
-target_link_libraries(python PRIVATE linenoise cxxopts python-cpp project_options project_warnings stdc++)
+target_link_libraries(python PRIVATE linenoise cxxopts python-cpp stdc++)
 
 add_executable(freeze utilities/freeze.cpp)
-target_link_libraries(freeze PRIVATE python-cpp cxxopts project_options project_warnings)
+target_link_libraries(freeze PRIVATE python-cpp cxxopts)
 target_include_directories(freeze SYSTEM PRIVATE ${MLIR_INCLUDE_DIRS})
+
+python_cpp_link_project_options(unittests_ python freeze)

src/executable/mlir/CMakeLists.txt

@@ -27,6 +27,23 @@ include(AddLLVM)
 set(PYTHON_MLIR_SOURCE_DIR ${PROJECT_SOURCE_DIR}/src/executable/mlir)
 set(PYTHON_MLIR_BINARY_DIR ${PROJECT_BINARY_DIR}/src/executable/mlir)
 
+# The LLVM/MLIR helpers used below (add_mlir_library and friends) compile their
+# sources in a separate `obj.<name>` object library and forward the target's
+# INCLUDE_DIRECTORIES to it as plain include paths, which drops the SYSTEM
+# marking. Marking the third-party headers as system directories for the whole
+# subtree survives that, and keeps our warning set (-Werror included) from
+# firing inside LLVM, MLIR and spdlog headers.
+#
+# ${PYTHON_MLIR_BINARY_DIR} holds nothing but TableGen output (Ops.h.inc,
+# Passes.h.inc, ...), which is machine-generated and not ours to clean up, so it
+# is treated the same way.
+include_directories(SYSTEM
+                    ${LLVM_INCLUDE_DIRS}
+                    ${MLIR_INCLUDE_DIRS}
+                    ${spdlog_SOURCE_DIR}/include
+                    ${PYTHON_MLIR_BINARY_DIR}
+                    ${PYTHON_MLIR_BINARY_DIR}/Dialect)
+
 add_subdirectory(Conversion)
 add_subdirectory(Dialect)
 add_subdirectory(Target)
@@ -35,3 +52,4 @@ add_subdirectory(test)
 
 add_library(python-mlir compile.cpp)
 target_link_libraries(python-mlir PRIVATE PythonMLIRDialect TargetPythonBytecode PythonConversionPasses)
+python_cpp_link_project_options(python-mlir)

src/executable/mlir/Conversion/CMakeLists.txt

@@ -15,4 +15,6 @@ add_mlir_library(PythonConversionPasses
 
     LINK_LIBS PUBLIC
     ${PYTHON_CONVERSION_LIBS}
-)
+)
+
+python_cpp_link_project_options(PythonConversionPasses)

src/executable/mlir/Conversion/PythonToPythonBytecode/CMakeLists.txt

@@ -27,4 +27,5 @@ target_include_directories(PythonToPythonBytecode PUBLIC
     ${PYTHON_MLIR_BINARY_DIR}
 )
 
-target_link_libraries(PythonToPythonBytecode PRIVATE spdlog)
+target_link_libraries(PythonToPythonBytecode PRIVATE spdlog)
+python_cpp_link_project_options(PythonToPythonBytecode)

src/executable/mlir/Conversion/PythonToPythonBytecode/CollectionPatterns.cpp

@@ -36,22 +36,22 @@ namespace {
 					llvm::zip(op.getKeys(), op.getValues(), op.getRequiresExpansion())) {
 					if (to_expand) {
 						if (!result.has_value()) {
-							result = rewriter.create<mlir::emitpybytecode::BuildDict>(
-								op.getLoc(), op.getOutput().getType(), keys, values);
+							result = mlir::emitpybytecode::BuildDict::create(
+								rewriter, op.getLoc(), op.getOutput().getType(), keys, values);
 							keys.clear();
 							values.clear();
 						}
-						rewriter.create<mlir::emitpybytecode::DictUpdate>(
-							op.getLoc(), *result, value);
+						mlir::emitpybytecode::DictUpdate::create(
+							rewriter, op.getLoc(), *result, value);
 					} else {
 						if (!result.has_value()) {
 							keys.push_back(key);
 							values.push_back(value);
 						} else {
 							ASSERT(keys.empty());
 							ASSERT(values.empty());
-							rewriter.create<mlir::emitpybytecode::DictAdd>(
-								op.getLoc(), *result, key, value);
+							mlir::emitpybytecode::DictAdd::create(
+								rewriter, op.getLoc(), *result, key, value);
 						}
 					}
 				}
@@ -89,12 +89,12 @@ namespace {
 		llvm::ArrayRef<bool> requires_expansion)
 	{
 		auto list =
-			rewriter.create<mlir::emitpybytecode::BuildList>(loc, list_type, mlir::ValueRange{});
+			mlir::emitpybytecode::BuildList::create(rewriter, loc, list_type, mlir::ValueRange{});
 		for (auto [el, expand] : llvm::zip(elements, requires_expansion)) {
 			if (expand) {
-				rewriter.create<mlir::emitpybytecode::ListExtend>(loc, list, el);
+				mlir::emitpybytecode::ListExtend::create(rewriter, loc, list, el);
 			} else {
-				rewriter.create<mlir::emitpybytecode::ListAppend>(loc, list, el);
+				mlir::emitpybytecode::ListAppend::create(rewriter, loc, list, el);
 			}
 		}
 		return list;
@@ -181,23 +181,23 @@ namespace {
 				for (auto [el, expand] : llvm::zip(op.getElements(), requires_expansion)) {
 					if (expand) {
 						if (!set.has_value()) {
-							set = rewriter.create<mlir::emitpybytecode::BuildSet>(
-								op->getLoc(), op.getOutput().getType(), elements);
+							set = mlir::emitpybytecode::BuildSet::create(
+								rewriter, op->getLoc(), op.getOutput().getType(), elements);
 						} else {
 							for (auto el : elements) {
-								rewriter.create<mlir::emitpybytecode::SetAdd>(
-									op.getLoc(), *set, el);
+								mlir::emitpybytecode::SetAdd::create(
+									rewriter, op.getLoc(), *set, el);
 							}
 						}
 						elements.clear();
-						rewriter.create<mlir::emitpybytecode::SetUpdate>(op.getLoc(), *set, el);
+						mlir::emitpybytecode::SetUpdate::create(rewriter, op.getLoc(), *set, el);
 					} else {
 						elements.push_back(el);
 					}
 				}
 				ASSERT(set.has_value());
 				for (auto el : elements) {
-					rewriter.create<mlir::emitpybytecode::SetAdd>(op.getLoc(), *set, el);
+					mlir::emitpybytecode::SetAdd::create(rewriter, op.getLoc(), *set, el);
 				}
 				rewriter.replaceOp(op, *set);
 			} else {

src/executable/mlir/Conversion/PythonToPythonBytecode/ControlFlowPatterns.cpp

@@ -97,8 +97,8 @@ namespace {
 					auto insertion_point = rewriter.getInsertionPoint();
 					auto *return_block = rewriter.createBlock(&op.getRegion());
 					auto value =
-						rewriter.create<mlir::py::ConstantOp>(op.getLoc(), rewriter.getNoneType());
-					rewriter.create<mlir::func::ReturnOp>(op.getLoc(), mlir::ValueRange{ value });
+						mlir::py::ConstantOp::create(rewriter, op.getLoc(), rewriter.getNoneType());
+					mlir::func::ReturnOp::create(rewriter, op.getLoc(), mlir::ValueRange{ value });
 					rewriter.setInsertionPoint(insertion_point->getBlock(), insertion_point);
 					return return_block;
 				})
@@ -136,9 +136,10 @@ namespace {
 		mlir::LogicalResult matchAndRewrite(mlir::py::YieldFromOp op,
 			mlir::PatternRewriter &rewriter) const final
 		{
-			auto iterator = rewriter.create<mlir::emitpybytecode::YieldFromIter>(
-				op.getLoc(), op.getIterable().getType(), op.getIterable());
-			auto value = rewriter.create<mlir::py::ConstantOp>(op.getLoc(), rewriter.getNoneType());
+			auto iterator = mlir::emitpybytecode::YieldFromIter::create(
+				rewriter, op.getLoc(), op.getIterable().getType(), op.getIterable());
+			auto value =
+				mlir::py::ConstantOp::create(rewriter, op.getLoc(), rewriter.getNoneType());
 
 			rewriter.replaceOpWithNewOp<mlir::emitpybytecode::YieldFrom>(
 				op, iterator.getType(), iterator, value);