Chiming in: they specifically refer to mmCOMPUTE_USER_DATA_0 and mmSPI_SHADER_USER_DATA_{STAGE}_0, which are in the public chip register offsets (not anywhere in LLVM right now, but present in PAL, Mesa, and any public chip register spec). I don't think we should explicitly define those values here, but using those names might make it a bit more specific and easier to find.

Belated 'sounds good to me'.

I'd be very surprised if any of the tests included in this change pass with that line commented.... it's meant so that things like #if and #else properly separate alignment after the first preprocessor run, where the whitespace manager doesn't have the full context of things between it.

After looking at more of the history (ie, the commit you referenced), I'd definitely be open to something like this, provided that it doesn't affect namespaces that reside completely on one line. Since it was mostly a clang-format limitation and relatively rare, I think we can change the default here, but that's not up to just me (+@MyDeveloperDay), and extra scrutiny is definitely required when changing existing tests.

Sorry on the delay, LGTM too.

I can see the use of this, but I am also wary that ignoring style options will lead to people producing different results on different versions of clang-format. This is both because having set-or-unset an option will naturally lead to different code and also that newer options are a de-facto check that clang-format is at least a certain version (we have minor differences between major versions as bugs are fixed). In any case, I see this being *very* easy to misuse and the documentation should have a warning reflecting that.

In D86713#2242300, @arichardson wrote:

In D86713#2242253, @JakeMerdichAMD wrote:

LGTM, again assuming tests pass locally (patch did not resolve).

Out of curiosity, is _Atomic on your radar? I found some code in clang proper that handled restrict and _Atomic together. C/C++ have way too many qualifiers...

I have not looked at _Atomic yet and it's probably low priority for me unless it's a trivial change.
My main motivation with these changes is to format a __capability pointer qualifier correctly (an extension that we add for our out-of-tree CHERI C/C++ dialect).

LGTM, again assuming tests pass locally (patch did not resolve).

LGTM, assuming tests pass (automated checks failed to resolve your patch since you based it off of your other one). Looks like enabling C99 should have no other effects, right?

LGTM. Wait a bit to give others a chance to chime in before submitting.

Agreed that this is a very nice solution for this case. LGTM too assuming it passes @MyDeveloperDay's tests.

Reviving this since it looks perfectly fine to me (from my limited commit history in git-clang-format :P), is useful, and there's no good reason for it to be stalled.

Thanks for the fast review, @curdeius, and thanks for mentioning PP_STRINGIZE and BOOST_PP_STRINGIZE too!

Address feedback (nits, better docs, more defaults)

Great idea on this! I may borrow this idea and make something similar for some migrations I'm working on.

I'm a fan of the 'like' helpers, but I'm not entirely convinced that having helpers for languages covered by the 'like' is a bad thing-- it just needs to be very explicit that you do mean only that language. For example, an 'isObjCOnly()' would hint to reviewers that ObjC is sometimes used in combination with other languages and there may be a more appropriate helper (of course, this should be clearly documented as well).

LGTM

Just belatedly caught something: Webkit style is supported too but not listed here. Can you add that?

+1 for this idea. It'd eventually be neat to also take all samples from the style guide of each project and test them, if there aren't licensing concerns.

Hey @MyDeveloperDay, can I get your assistance committing this when you have the chance?

Can I get your assistance committing this?

Reformat with a newer clang-format

LGTM assuming CI tests pass.

See follow-up in D80176.

This is a great improvement in readability. I think this will get in here before it does for clang proper :D

I mainly have concerns with the search for noexcept being too brittle and not looking far enough back. Implementing that may have performance implications though, so I have no issue ignoring that problem if it's proven sufficiently rare.

Good idea, feel free to incorporate anything about the change (eg, tests and TT_BitFieldColon) or delegate to me if you like.

LGTM

Ironically, I independently made a near-identical change because my codebase needs this, and was going to upload it today. I won't post it now, but I stuck a copy on github if you're interested: https://github.com/JakeMerdichAMD/llvm-project/commit/6eec9ce0bb7732a519b765659422b72a9fa8aa67. I also have some more tests there, and will cross-check with this once the merge conflicts are fixed.

Rebase to fix merge conflict

Add a comment explaining why checking IsInsideToken is needed here.

One spelling nit, but otherwise looks good to me.

Thanks for the commit and review @MyDeveloperDay!

Hey, @MyDeveloperDay, can I get your assistance in committing this? It's probably been long enough for anyone to chime in.

Mind looking again? They did add one later on I think. It helped a lot since the exact settings and whitespace to trigger this are really finicky (though reproducible). You're right that it won't reproduce without it.

The failing case in this commit looks like the following after formatting (with alignconsecutiveassignments and a specific column limit)

Sure, I'll get started on that. It mainly comes from charging headfirst into the edge cases, but I think I have a decent grasp of clang-format internals now, and I'm definitely interested in both contributing to and reviewing for it.

@MyDeveloperDay, you're right about what this issue addresses: it surprised me a lot when an unrelated edit caused something to 'randomly' add spaces elsewhere, since it's in a different ifdef block (whether or not it has the same condition). The code that's aligned should always be across lines that are visually consecutive, and those ones weren't (in most cases, they aren't even logically consecutive).