[Mlir-commits] [clang] [clang-tools-extra] [compiler-rt] [flang] [lld] [lldb] [llvm] [mlir] [openmp] [pstl] Finally formalise our defacto line-ending policy (PR #86318)

[llvmlistbot at llvm.org]

ldrumm wrote:

> There are a bunch of tests that fail if you don't.

Yes. Even better than that: this patch just uncovered a [legitimate bug in the C++ AST parser tests for clang

> Our instructions tell people they need to disable autocrlf

I didn't see that. Shall I remove that instruction now, or after we've merged?

> I don't know that this PR is perfect, but IMO it is a huge step in the right direction to allow Windows contributors to build, test and contribute to LLVM using the default configurations of the OS and tools.

Absolutely. This patch is not a panacea but it enables us to correctly formalize the case where we actually *do* care about line-endings. Before this patch, it was basically incredibly brittle, but now we actually have control for those tests that matter. For things that don't really matter a whole class of irritating paper-cuts go away forever.

I understand folks' hesitance around downstream rebases, but since this will go in as two commits, resolving a merge conflict becomes an easy step (I've [given instructions in the commit that will cause conflicts](https://github.com/llvm/llvm-project/pull/86318/commits/0b70d26534ac788741dc412777fa3396f8c1407c)). I've bumped downstream llvm projects through many major releases in my life (llvm v3 through 13), and change-of-method-name has been a much greater source of merge conflicts than whitespace. LLVM's codebase is healthy and clean largely because we give people the ability to making sweeping improvements. In any case, this will be the last time that a downstream will need to resolve major CRLF changes during a bump, so I consider that a long-term win too.

https://github.com/llvm/llvm-project/pull/86318