Skip to content

Enable -Werror in CMake (CI only) #9037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alexreinking merged 9 commits into from
Mar 16, 2026
Merged

Enable -Werror in CMake (CI only) #9037

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4822577
Enable -Werror in CMake (CI only)
alexreinking Mar 15, 2026
c7c4bf8
Fix unused variable warning for has_scalable_vector in Target.cpp
alexreinking Mar 15, 2026
e87a146
Suppress unsafe CRT function warnings in halide_image_io.h
alexreinking Mar 15, 2026
82b8ea1
Fix loss of precision in double-to-float conversion
alexreinking Mar 15, 2026
feb2003
Suppress unsafe CRT function warnings in test_sharding.h
alexreinking Mar 15, 2026
a6afb47
Set _CRT_SECURE_NO_WARNINGS for Halide::Test sources
alexreinking Mar 15, 2026
b746b5b
Fix float literal suffix.
alexreinking Mar 15, 2026
c12e80a
MSVC: cast M_PI to float (C4305)
alexreinking Mar 15, 2026
eafcda7
Normalize line endings in templates and string literals
alexreinking Mar 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CMakePresets.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"toolchainFile": "$env{VCPKG_INSTALLATION_ROOT}/scripts/buildsystems/vcpkg.cmake",
"cacheVariables": {
"CMAKE_BUILD_TYPE": "RelWithDebInfo",
"CMAKE_COMPILE_WARNING_AS_ERROR": "ON",
"FETCHCONTENT_FULLY_DISCONNECTED": "ON",
"VCPKG_CHAINLOAD_TOOLCHAIN_FILE": "${sourceDir}/cmake/toolchain.${presetName}.cmake",
"VCPKG_MANIFEST_FEATURES": "developer",
Expand Down
5 changes: 5 additions & 0 deletions cmake/HalideTestHelpers.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ if (NOT TARGET Halide::Test)
# Make internal_assert, debug, etc. available to tests
target_compile_definitions(Halide_test INTERFACE HALIDE_KEEP_MACROS)

# Disable warnings about standard C functions that have more secure replacements
# in the Windows API.
target_compile_definitions(Halide_test INTERFACE
$<$<CXX_COMPILER_ID:MSVC>:_CRT_SECURE_NO_WARNINGS>)

# Everyone gets to see the common headers
target_include_directories(Halide_test
INTERFACE
Expand Down
81 changes: 47 additions & 34 deletions src/CodeGen_C.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,29 @@ extern "C" unsigned char halide_c_template_CodeGen_C_vectors[];

namespace {

// On Windows, raw string literals and binary2cpp-generated arrays can
// contain \r\n line endings (from git checkout with core.autocrlf).
// Normalize to \n so that generated source files have consistent,
// platform-independent line endings.
Copy link
Copy Markdown
Contributor

@mcourteaux mcourteaux Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wauw this sucks. We couldn't just normalize those source files?

Copy link
Copy Markdown
Member Author

@alexreinking alexreinking Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's brittle. People's editors might change things locally and ephemerally before git renormalizes them. When we upgrade to C++20, this can be consteval

Copy link
Copy Markdown
Contributor

@mcourteaux mcourteaux Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No chance for a pre-commit check?

Copy link
Copy Markdown
Member Author

@alexreinking alexreinking Mar 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't help during local development. The scenario I'm envisioning is that someone edits CodeGen_C.cpp on Windows when it's checked out by git as LF. Upon save, their editor changes it silently to CRLF. I would hope most editors are smart about this, but who knows? Maybe someone is doing something weird like running VSCode in a WSL container yet compiling on Windows. Now there's a mix of line endings in the generated sources that will be very hard to track down.

I thought of a few alternative approaches but this seemed like the lowest-friction option:

  1. We could switch the output to binary mode. But then everywhere we write a '\n' character today would need to be replaced with Halide::Endl or something that expands to \n on Unix and \r\n on Windows.
  2. We could add a -text flag to binary2cpp that normalizes newlines in the template files. Doesn't fix the problem with raw string literals, though
  3. I did think of changing the line endings of the relevant files with .gitattributes (which I think is what you were suggesting?)---but that's brittle for the reasons I just described. I don't like that it's outside the code. For example, what happens if someone downloads a ZIP of our sources from GitHub? Maybe it works, but I don't want to have to think about it or rely on it.

It is incredibly vexing that the standard did not define the bytes of a newline in a raw string literal.

string normalize_line_endings(const char *s) {
string result;
result.reserve(strlen(s));
for (; *s; ++s) {
if (*s != '\r') {
result += *s;
}
}
return result;
}

string normalize_line_endings(const unsigned char *s) {
return normalize_line_endings(reinterpret_cast<const char *>(s));
}

// HALIDE_MUST_USE_RESULT defined here is intended to exactly
// duplicate the definition in HalideRuntime.h (so that either or
// both can be present, in any order).
const char *const kDefineMustUseResult = R"INLINE_CODE(#ifndef HALIDE_MUST_USE_RESULT
const string kDefineMustUseResult = normalize_line_endings(R"INLINE_CODE(#ifndef HALIDE_MUST_USE_RESULT
#ifdef __has_attribute
#if __has_attribute(nodiscard)
#define HALIDE_MUST_USE_RESULT [[nodiscard]]
Expand All @@ -55,9 +74,9 @@ const char *const kDefineMustUseResult = R"INLINE_CODE(#ifndef HALIDE_MUST_USE_R
#define HALIDE_MUST_USE_RESULT
#endif
#endif
)INLINE_CODE";
)INLINE_CODE");

const char *const constexpr_argument_info_docs = R"INLINE_CODE(
const string constexpr_argument_info_docs = normalize_line_endings(R"INLINE_CODE(
/**
* This function returns a constexpr array of information about a Halide-generated
* function's argument signature (e.g., number of arguments, type of each, etc).
Expand Down Expand Up @@ -88,7 +107,7 @@ const char *const constexpr_argument_info_docs = R"INLINE_CODE(
* impact aside from the numerical value of the constant.
*/

)INLINE_CODE";
)INLINE_CODE");

class TypeInfoGatherer : public IRGraphVisitor {
private:
Expand Down Expand Up @@ -194,7 +213,7 @@ CodeGen_C::CodeGen_C(ostream &s, const Target &t, OutputKind output_kind, const
// If it's a header, emit an include guard.
stream << "#ifndef HALIDE_FUNCTION_INFO_" << c_print_name(guard) << "\n"
<< "#define HALIDE_FUNCTION_INFO_" << c_print_name(guard) << "\n";
stream << R"INLINE_CODE(
stream << normalize_line_endings(R"INLINE_CODE(
/* MACHINE GENERATED By Halide. */

#if !(__cplusplus >= 201703L || _MSVC_LANG >= 201703L)
Expand All @@ -203,7 +222,7 @@ CodeGen_C::CodeGen_C(ostream &s, const Target &t, OutputKind output_kind, const

#include "HalideRuntime.h"

)INLINE_CODE";
)INLINE_CODE");

return;
}
Expand Down Expand Up @@ -247,9 +266,9 @@ CodeGen_C::CodeGen_C(ostream &s, const Target &t, OutputKind output_kind, const
} else {
// Include declarations of everything generated C source might want
stream
<< halide_c_template_CodeGen_C_prologue << "\n"
<< halide_internal_runtime_header_HalideRuntime_h << "\n"
<< halide_internal_initmod_inlined_c << "\n";
<< normalize_line_endings(halide_c_template_CodeGen_C_prologue) << "\n"
<< normalize_line_endings(halide_internal_runtime_header_HalideRuntime_h) << "\n"
<< normalize_line_endings(halide_internal_initmod_inlined_c) << "\n";
stream << "\n";
}

Expand Down Expand Up @@ -294,24 +313,24 @@ CodeGen_C::~CodeGen_C() {
<< "// use the -r flag with any Halide generator binary, e.g.:\n"
<< "// $ ./my_generator -r halide_runtime -o . target=host\n"
<< "\n"
<< halide_internal_runtime_header_HalideRuntime_h << "\n";
<< normalize_line_endings(halide_internal_runtime_header_HalideRuntime_h) << "\n";
if (target.has_feature(Target::CUDA)) {
stream << halide_internal_runtime_header_HalideRuntimeCuda_h << "\n";
stream << normalize_line_endings(halide_internal_runtime_header_HalideRuntimeCuda_h) << "\n";
}
if (target.has_feature(Target::HVX)) {
stream << halide_internal_runtime_header_HalideRuntimeHexagonHost_h << "\n";
stream << normalize_line_endings(halide_internal_runtime_header_HalideRuntimeHexagonHost_h) << "\n";
}
if (target.has_feature(Target::Metal)) {
stream << halide_internal_runtime_header_HalideRuntimeMetal_h << "\n";
stream << normalize_line_endings(halide_internal_runtime_header_HalideRuntimeMetal_h) << "\n";
}
if (target.has_feature(Target::OpenCL)) {
stream << halide_internal_runtime_header_HalideRuntimeOpenCL_h << "\n";
stream << normalize_line_endings(halide_internal_runtime_header_HalideRuntimeOpenCL_h) << "\n";
}
if (target.has_feature(Target::D3D12Compute)) {
stream << halide_internal_runtime_header_HalideRuntimeD3D12Compute_h << "\n";
stream << normalize_line_endings(halide_internal_runtime_header_HalideRuntimeD3D12Compute_h) << "\n";
}
if (target.has_feature(Target::WebGPU)) {
stream << halide_internal_runtime_header_HalideRuntimeWebGPU_h << "\n";
stream << normalize_line_endings(halide_internal_runtime_header_HalideRuntimeWebGPU_h) << "\n";
}
}
stream << "#endif\n";
Expand All @@ -324,13 +343,7 @@ void CodeGen_C::add_platform_prologue() {

void CodeGen_C::add_vector_typedefs(const std::set<Type> &vector_types) {
if (!vector_types.empty()) {
// Voodoo fix: on at least one config (our arm32 buildbot running gcc 5.4),
// emitting this long text string was regularly garbled in a predictable pattern;
// flushing the stream before or after heals it. Since C++ codegen is rarely
// on a compilation critical path, we'll just band-aid it in this way.
stream << std::flush;
stream << halide_c_template_CodeGen_C_vectors;
stream << std::flush;
stream << normalize_line_endings(halide_c_template_CodeGen_C_vectors);

for (const auto &t : vector_types) {
string name = print_type(t, DoNotAppendSpace);
Expand All @@ -354,20 +367,20 @@ void CodeGen_C::add_vector_typedefs(const std::set<Type> &vector_types) {

void CodeGen_C::set_name_mangling_mode(NameMangling mode) {
if (extern_c_open && mode != NameMangling::C) {
stream << R"INLINE_CODE(
stream << normalize_line_endings(R"INLINE_CODE(
#ifdef __cplusplus
} // extern "C"
#endif

)INLINE_CODE";
)INLINE_CODE");
extern_c_open = false;
} else if (!extern_c_open && mode == NameMangling::C) {
stream << R"INLINE_CODE(
stream << normalize_line_endings(R"INLINE_CODE(
#ifdef __cplusplus
extern "C" {
#endif

)INLINE_CODE";
)INLINE_CODE");
extern_c_open = true;
}
}
Expand Down Expand Up @@ -2567,10 +2580,10 @@ void CodeGen_C::test() {
}

string correct_source =
string((const char *)halide_c_template_CodeGen_C_prologue) + '\n' +
string((const char *)halide_internal_runtime_header_HalideRuntime_h) + '\n' +
string((const char *)halide_internal_initmod_inlined_c) + '\n' +
'\n' + kDefineMustUseResult + R"GOLDEN_CODE(
normalize_line_endings(halide_c_template_CodeGen_C_prologue) + '\n' +
normalize_line_endings(halide_internal_runtime_header_HalideRuntime_h) + '\n' +
normalize_line_endings(halide_internal_initmod_inlined_c) + '\n' +
'\n' + kDefineMustUseResult + normalize_line_endings(R"GOLDEN_CODE(
#ifndef HALIDE_FUNCTION_ATTRS
#define HALIDE_FUNCTION_ATTRS
#endif
Expand Down Expand Up @@ -2638,7 +2651,7 @@ int test1(struct halide_buffer_t *_buf_buffer, float _alpha, int32_t _beta, void
} // extern "C"
#endif

)GOLDEN_CODE";
)GOLDEN_CODE");

const auto compare_srcs = [](const string &actual, const string &expected) {
if (actual != expected) {
Expand Down Expand Up @@ -2673,7 +2686,7 @@ int test1(struct halide_buffer_t *_buf_buffer, float _alpha, int32_t _beta, void
cg.compile(m);
}

string correct_function_info = R"GOLDEN_CODE(#ifndef HALIDE_FUNCTION_INFO__Function___Info___Test
string correct_function_info = normalize_line_endings(R"GOLDEN_CODE(#ifndef HALIDE_FUNCTION_INFO__Function___Info___Test
#define HALIDE_FUNCTION_INFO__Function___Info___Test

/* MACHINE GENERATED By Halide. */
Expand Down Expand Up @@ -2724,7 +2737,7 @@ inline constexpr std::array<::HalideFunctionInfo::ArgumentInfo, 4> test1_argumen
}};
}
#endif
)GOLDEN_CODE";
)GOLDEN_CODE");

compare_srcs(function_info.str(), correct_function_info);

Expand Down
20 changes: 16 additions & 4 deletions src/PythonExtensionGen.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,18 @@ using std::string;

namespace {

// See normalize_line_endings in CodeGen_C.cpp for rationale.
string normalize_line_endings(const char *s) {
string result;
result.reserve(strlen(s));
for (; *s; ++s) {
if (*s != '\r') {
result += *s;
}
}
return result;
}

string sanitize_name(const string &name) {
ostringstream oss;
for (char c : name) {
Expand Down Expand Up @@ -95,7 +107,7 @@ std::pair<string, string> print_type(const LoweredArgument *arg) {
}
}

const char kModuleRegistrationCode[] = R"INLINE_CODE(
const string kModuleRegistrationCode = normalize_line_endings(R"INLINE_CODE(
static_assert(PY_MAJOR_VERSION >= 3, "Python bindings for Halide require Python 3+");

namespace Halide::PythonExtensions {
Expand Down Expand Up @@ -271,7 +283,7 @@ HALIDE_EXPORT_SYMBOL PyObject *_HALIDE_EXPAND_AND_CONCAT(PyInit_, HALIDE_PYTHON_
}

} // extern "C"
)INLINE_CODE";
)INLINE_CODE");

} // namespace

Expand Down Expand Up @@ -300,7 +312,7 @@ void PythonExtensionGen::compile(const Module &module) {
extern_decl_gen.compile(module);
}

dest << R"INLINE_CODE(
dest << normalize_line_endings(R"INLINE_CODE(
namespace Halide::PythonRuntime {
extern bool unpack_buffer(PyObject *py_obj,
int py_getbuffer_flags,
Expand Down Expand Up @@ -389,7 +401,7 @@ struct PyHalideBuffer {

} // namespace

)INLINE_CODE";
)INLINE_CODE");

for (const auto &f : module.functions()) {
if (f.linkage == LinkageType::ExternalPlusMetadata) {
Expand Down
8 changes: 7 additions & 1 deletion src/Target.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,9 @@ Target calculate_host_target() {
#else
#if defined(__arm__) || defined(__aarch64__) || defined(_M_ARM64) || defined(_M_ARM64EC)
Target::Arch arch = Target::ARM;
#if !defined(__arm__)
bool has_scalable_vector = false;
#endif

#ifdef __APPLE__
if (is_armv7s()) {
Expand Down Expand Up @@ -293,7 +295,9 @@ Target calculate_host_target() {

if (hwcaps2 & HWCAP2_SVE2) {
initial_features.push_back(Target::SVE2);
#if !defined(__arm__)
has_scalable_vector = true;
#endif
}
#endif

Expand Down Expand Up @@ -322,12 +326,14 @@ Target calculate_host_target() {

if (IsProcessorFeaturePresent(PF_ARM_SVE2_INSTRUCTIONS_AVAILABLE)) {
initial_features.push_back(Target::SVE2);
#if !defined(__arm__)
has_scalable_vector = true;
#endif
}

#endif

#if defined(__aarch64__)
#if !defined(__arm__)
if (has_scalable_vector) {
vector_bits = get_sve_vector_length();
}
Expand Down
2 changes: 1 addition & 1 deletion test/autoschedulers/anderson2021/test.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ void expect_eq(int line, const A &expected, const B &actual) {
}

template<typename A, typename B>
void approx_eq(int line, const A &expected, const B &actual, float epsilon) {
void approx_eq(int line, const A &expected, const B &actual, double epsilon) {
user_assert(std::abs(expected - actual) < epsilon)
<< "Assert failed on line " << line << "."
<< "\nExpected value = " << expected
Expand Down
9 changes: 9 additions & 0 deletions test/common/test_sharding.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
#ifndef TEST_SHARDING_H
#define TEST_SHARDING_H

#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable : 4996)
#endif

// This file may be used by AOT tests, so it deliberately does not
// include Halide.h

Expand Down Expand Up @@ -86,4 +91,8 @@ class Sharder {
} // namespace Internal
} // namespace Halide

#ifdef _MSC_VER
#pragma warning(pop)
#endif

#endif // TEST_SHARDING_H
2 changes: 1 addition & 1 deletion test/correctness/fuzz_schedule.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ int main(int argc, char **argv) {
// https://github.com/halide/Halide/issues/8054
{
ImageParam input(Float(32), 2, "input");
const float r_sigma = 0.1;
const float r_sigma = 0.1f;
const int s_sigma = 8;
Func bilateral_grid{"bilateral_grid"};

Expand Down
4 changes: 2 additions & 2 deletions test/correctness/rfactor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -839,7 +839,7 @@ enum class InlineReductionVariant {
template<InlineReductionVariant variant>
int inline_reductions_test() {
using namespace ConciseCasts;
constexpr float pi = M_PI;
constexpr float pi = static_cast<float>(M_PI);

Func f{"f"};
Var x("x");
Expand Down Expand Up @@ -897,7 +897,7 @@ enum class ArgMaxTupleOrder {
template<ArgMaxVariant variant, ArgMaxTupleOrder order>
int argmax_rfactor_test() {
using namespace ConciseCasts;
constexpr float pi = M_PI;
constexpr float pi = static_cast<float>(M_PI);

Func f{"f"};
Var x("x");
Expand Down
9 changes: 9 additions & 0 deletions tools/halide_image_io.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,11 @@
#ifndef HALIDE_IMAGE_IO_H
#define HALIDE_IMAGE_IO_H

#ifdef _MSC_VER
#pragma warning(push)
#pragma warning(disable : 4996) // disable unsafe CRT function warnings
#endif

#include <algorithm>
#include <cctype>
#include <cmath>
Expand Down Expand Up @@ -2791,4 +2796,8 @@ void convert_and_save_image(ImageType &im, const std::string &filename) {
} // namespace Tools
} // namespace Halide

#ifdef _MSC_VER
#pragma warning(pop)
#endif

#endif // HALIDE_IMAGE_IO_H
Loading