You might also mention that this solves #54716 (in my understanding).

Looks good to me.

In D125429#3512642, @aaronpuchert wrote:

Proposed this in D125580.

In D125429#3511648, @aaronpuchert wrote:

And it appears that unsigned long does not have a clear favorite here.

So we have overloads

Hmm, didn't see this on Linux. I'll try to disambiguate.

Unify Argument types and parsing. Add AST test. Simplify TableGen.

Add an AST test to check that we parse the arguments correctly. (We don't use them for diagnostics currently.)

Ping.

Add a release note.

In D124966#3497305, @aaron.ballman wrote:

LGTM! You should probably add a release note for this change so users know about it.

Use 1 instead of 0 just to be sure. We don't want to trigger warnings about division by zero.

Also consider ++ and -- as writing, test all operators.

@rupprecht, do you still do integration at Google? This patch might be interesting to try out, since it strengthens some warnings.

In D124153#3480334, @kippesp wrote:

If CMAKE_BUILD_TYPE is specified in a multi-configuration build (like visual studio), the alternative build types won't be available.

In D124606#3480281, @labath wrote:

In D124606#3480164, @aaronpuchert wrote:

An editor could inadvertently change the line endings, and someone might not notice before committing.

From what I understood, most of these tests would immediately break (due to hardcoded offsets) if the line endings were changed.

Ok, that's a valid point.

Also I still don't understand what specifically this is fixing. What exactly was wrong about the previous configuration?

Fair enough, but don't we want to enforce LF or CRLF, respectively? An editor could inadvertently change the line endings, and someone might not notice before committing. Explicitly specifying the right line endings gives us the proper line endings even if some editors mess things up.

I think this change broke again what D97625 was trying to fix. These are text files and we want them to keep the specified line endings regardless of the local git configuration. See also @smeenai's comment D124563#3478625.

In D124563#3479819, @MForster wrote:

See D124606. -text seems to be the right compromise.

As a temporary workaround for switching between branches, just add a local file clang-tools-extra/test/clang-apply-replacements/.gitattributes with contents:

Inputs/crlf/crlf.cpp binary
Inputs/crlf/crlf.cpp.expected binary

This will remove all kinds of filtering for the moment. When you remove that file, just make sure to force checkout both files again to make sure the filter applies.

In D124563#3479373, @nikic wrote:

This change seems to have broken switching to a branch before the commit completely. The only way I was able to recover my git checkout is to comment out the * text=auto line and then run git reset --hard, otherwise the crlf.cpp files always show up as changed.

In D124563#3478978, @aaronpuchert wrote:

Git will apply the LF -> CRLF transformation when it checks out itself. Git doesn't show the file as modified because after cleaning the file (i.e. applying CRLF -> LF) it's the same as in the index.

In D124563#3478968, @modimo wrote:

I used arc patch and also saw the same thing.

In D124563#3478937, @modimo wrote:

Checking locally I'm seeing LF as the line ending in crlf.cpp in the working directory. Can you double check that everything matches up?

Ok, let's do it this way.

Would be nice if someone could accept this. I think this comes closest to the intention of D97625.

Drop * text=auto, so that we renormalize only the files that need it.

In D124563#3478653, @modimo wrote:

I think the way to go is to revert ac5f7be6a868 then land everything as a single stack to prevent this issue.

In D124563#3478747, @aaronpuchert wrote:

How about we remove * text=auto for now? Or would that break something?

In D124563#3478625, @smeenai wrote:

I *think* this would mean that if you're on Windows and have core.autocrlf set to input, when you commit changes to this files, Git will convert them back to LF line endings. Not 100% sure of that though.

Most of the files changed here are not affected by the .gitattributes file, such as

clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/readability-duplicate-include2.h
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-duplicate-include/system/sys/types.h
clang-tools-extra/test/clang-tidy/checkers/readability-else-after-return-if-constexpr.cpp
clang-tools-extra/test/modularize/Inputs/CompileError/module.modulemap
clang-tools-extra/test/modularize/Inputs/MissingHeader/module.modulemap
clang-tools-extra/test/pp-trace/Inputs/module.map

The only affected files seem to be

clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp
clang-tools-extra/test/clang-apply-replacements/Inputs/crlf/crlf.cpp.expected

This looks pretty good!

Are the files bit-by-bit identical between the checkouts? I guess it might be an issue in the index.

The build types control three different aspects, so maybe you could even present this as a table:

In D97625#3337018, @dmgreen wrote:

I'm not sure it will do much where it is.

In D124153#3468447, @zero9178 wrote:

• Linker OOMs are mainly caused by large object files, both due to code size of unoptimized code and especially due to the extra debug info. Hence the first recommendation in any of the forums when people show a linker OOM is to make sure they're using a release build, if that is what they intended, and to use LLD or Gold (that is a separate topic however)

In D124153#3466525, @tstellar wrote:

My experience tells me that if we present 3 options to choose from, many users will just randomly pick one without thinking about it, and we'll still get bug reports about debug builds using too much memory.

We could also point to https://llvm.org/docs/CMake.html if no CMAKE_BUILD_TYPE is given. Depending on the use case (e.g. debug info might actually be wanted) other flags might be helpful like BUILD_SHARED_LIBS or LLVM_USE_SPLIT_DWARF. Or if you're working on frontend topics, set LLVM_TARGETS_TO_BUILD=Native. Some of these have a greater effect on build time/size than CMAKE_BUILD_TYPE=Release in my experience, and the page lists these options quite nicely.

In D124153#3464356, @hans wrote:

• We can flip it to ON if you don't specify CMAKE_BUILD_TYPE and OFF if you pass Release and ON when passing debug. But this seems a bit ... convoluted? Is that really what end users expect?

Actually I think that's exactly how it works today :-)

I see your point about the logic getting convoluted, but I don't think it'd be too bad, especially since we're printing a message about it anyway. It could be "No build type selected, default to Release with asserts enabled".

In rGf10cee91ae07022e602d6a47e504e86796d49a7d#1072344, @njames93 wrote:

This fix doesn't look correct as filecheck will still look for the diagnostic on 32bit platforms. You would also need to use a check-prefix to disable the filecheck check line

Note that __int128_t is not available everywhere, generally only on 64-bit platforms.

Feel free to fix that on commit.

Just some suggestions, feel free to ignore them.

In D122090#3398182, @nickdesaulniers wrote:

I think it would only be safe to assume --rtlib=compiler-rt if the compiler was driving a full link. Otherwise, the linker could be invoked manually against libgcc.

In D122090#3394964, @aaronpuchert wrote:

I was also wondering if we could block the overflow intrinsics only if Subtarget.getTargetTriple().isGNUEnvironment() is true, but we can also discuss this separately.

Seems to have broken the documentation build:

Warning, treated as error:
/home/buildbot/llvm-build-dir/llvm-sphinx-docs/llvm/src/llvm/docs/MemorySSA.rst:168:Unknown target name: "build-time use optimization".

I was also wondering if we could block the overflow intrinsics only if Subtarget.getTargetTriple().isGNUEnvironment() is true, but we can also discuss this separately. (That would affect all targets then, I guess.)

Had to adapt two more tests where we were emitting __multi3 calls on PowerPC 32-bit. That builtin doesn't exist even in compiler-rt, so I guess it's an improvement.