Enable -Werror in CMake (CI only)#9037
Conversation
|
Actually... why CI only? I wanna know it will fail before I push. |
Well, to do that you'd have to start by installing one of the exact same versions of GCC/Clang/MSVC as the buildbots have and then test on all of them 😉 The key issue is that The only way to know it'll pass CI is to run CI. If you want to build more like CI, we have |
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.
|
Error is an intermittent timeout failure in the onnx app on win-worker-3 |
| // 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. |
There was a problem hiding this comment.
Wauw this sucks. We couldn't just normalize those source files?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
No chance for a pre-commit check?
There was a problem hiding this comment.
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:
- We could switch the output to binary mode. But then everywhere we write a '\n' character today would need to be replaced with
Halide::Endlor something that expands to\non Unix and\r\non Windows. - We could add a
-textflag to binary2cpp that normalizes newlines in the template files. Doesn't fix the problem with raw string literals, though - 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.
3 participants
Let's see what breaks!