Handles consecutive attributes too.

This patch is effectively NFC.

Don't format imaginary numbers.

In D146844#4220894, @rymiel wrote:

Could this possibly be an issue for more esoteric underscore-less UDLs, like i?
Does the code need to search for a suffix, could it not detect the absence of a digit? Sorry if the questions are silly, I haven't really looked at this part of the formatter in depth

In D146101#4204653, @jp4a50 wrote:

I think it's also worth noting that the google style guide gives an example of designated initializers indented at 2 spaces (whereas their "continuation indent" for wrapped function parameters is 4).

It's likely an error that the designated initializer example there shows 2-space indents as clang-format uses the 4-space continuation indent width:

This is possible, but isn't it also possible that they would prefer designated initializers to be indented at 2 spaces but don't have the option with clang-format currently?

@rymiel I'm deferring to you and @HazardyKnusperkeks, so doing nothing for now is fine.

Simplifies the fix.

In D146501#4209026, @MyDeveloperDay wrote:

Do we have any unit tests for this that we can pump a shed load of examples at?

@kadircet thanks for reporting the crash and reverting the commit. I will fix it and reland the patch.

Should we add tests to TokenAnnotatorTest.cpp?

In D146101#4201842, @jp4a50 wrote:

I understand the added complexity and maintenance burden of a new option but we do meet the 3 criteria listed in your link.

• it is part of the KJ style guide which is used by the capn proto project which has over 100 maintainers
• the style guide is publicly accessible here

Looks like this patch doesn't put the opening brace at the outerscope for some of the examples from the linked github issues:

$ cat .clang-format
AllowShortLambdasOnASingleLine: None
BraceWrapping:
  BeforeLambdaBody: true
BreakBeforeBraces: Custom
LambdaBodyIndentation: OuterScope
$ cat test.cpp
aaaaaaaaaaaaaaaaaaaa(1,
	             b(
	             	[]
                     {
                       return 0;
                     }));

In D141098#4192785, @mehdi_amini wrote:

In D141098#4111809, @owenpan wrote:

Can we just fix the buildbot so that it runs git-clang-format with --binary? Using clang-format-15 to format a patch to clang-format-16 source still looks wrong to me.

The general principle is that developers of LLVM 16 should be able to use clang-format from the previous release (that is: one where we have prebuilt binaries available) to format patches in general.
It does not seem reasonable to me that we should need to build clang-format ourselves for the purpose of writing patches to LLVM.

In D125171#4193996, @jrmolin wrote:

In D125171#4167866, @owenpan wrote:

Please see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.

I am doing this for my team, which writes the security endpoint for Elastic Defend. The code is currently private, though the binaries are free to download and run. The number of contributors is around 30, and the lines of code is in the 100Ks (around 500K). I have not found a way to accomplish what this does with the available options. I am adding tests and am happy to maintain this. It is a rather small addition, so it is quite simple to keep updated.

The previous implementation of the option involved a hack which corrupted the parenthesis state stack.

Please see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options. Is there a way to fix the issue without adding a new option?

In D145642#4186462, @rymiel wrote:

Could you please clarify what you mean by "regressions" here? Isn't the behaviour of this syntax broken to begin with? It doesn't change anything about lambdas without requires-clauses

In D145642#4186304, @rymiel wrote:

In D145642#4183539, @owenpan wrote:

Would this patch make the end result look worse without "making the requires clause formatting of lambdas
match the formatting for functions"? If yes, then we should not land it just yet.

Depends on your definition of "worse" :)
I outlined this on the github issue (https://github.com/llvm/llvm-project/issues/61269), essentially:

The result for the example provided on the github issue something like this:

[&]<typename Callable>(Callable&& callable)
  requires std::is_invocable_v<Callable>
{ static_cast<void>(callable); };

Unlike function declarations, lambdas with a requires clause still have the body on one line. I don't yet know how to go about changing this, and if I should at all. This of course isn't an issue if the lambda body has multiple lines.

I'm more concerned with the above case. If the body of the lambda (including the enclosing braces) should not be merged into a single line, then I would say the token-annotation-only fix might trigger regressions. Otherwise, we can safely land this patch now.

Would this patch make the end result look worse without "making the requires clause formatting of lambdas
match the formatting for functions"? If yes, then we should not land it just yet.

In D145344#4174634, @MyDeveloperDay wrote:

I would hazard a guess that this actually solves more issues than we realize.

In D145262#4168058, @jaredgrubb wrote:

I wasn't sure about testing (this is my first patch!) and the test-case I did was in clang/test/Format .. I can look at clang/unittests/Format and see how to model something like it.

If I do that, would that be in-addition-to or instead-of the test-case I included already?

Please add unit tests to relevant files in clang/unittests/Format/.

See also https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface.

Please see https://clang.llvm.org/docs/ClangFormatStyleOptions.html#adding-additional-style-options.

In D141230#4054768, @seanptmaher wrote:

I can't actually commit this -- could one of you? Or has it already been done? I remember reading that someone had to manually push the patch upstream.

Should we revert this patch?

In D144537#4152225, @rymiel wrote:

In D144537#4145545, @MyDeveloperDay wrote:

maybe we should cherry pick into 16?

I don't know how to do backports and I don't want to mess it up

In D144296#4137508, @HazardyKnusperkeks wrote:

And I see the style was bogus before, but shouldn't all variables start with a capital letter? Please fix that in the code you touch.

In D144296#4136133, @rymiel wrote:

Note: this patch also makes LevelIndentTracker::skipLine obsolete. I suppose I could clean that up in a following patch

Landed in c31bc7bdf810.

In D143755#4118802, @vedgy wrote:

Hi @owenpan. Thank you for fixing this bug!
Have you noticed this paragraph in my bug report?

I believe clang_getTypeSpelling(), or more likely QualType::print() used by it, should insert a tab character between such tokens to pretty-print compilable code. The tab character is preferable to the space character here, because the users may rely on the fact that pretty-printed binary operators are surrounded by spaces to distinguish them from angle brackets.

Added tryMergeGreaterGreater() to and fixed tryMergerLessLess() of FormatTokenLexer.

In D143546#4116354, @MyDeveloperDay wrote:

In D143546#4114341, @owenpan wrote:

In D143546#4113721, @rymiel wrote:

In D143546#4112077, @owenpan wrote:

As this one is an invalid-code-generation bug, I wanted it fixed ASAP.

Do you intend to backport it to the 16 release branch then?

I don't know but will go with whatever you, @MyDeveloperDay and @HazardyKnusperkeks prefer.

honestly, I trust your judgement @owenpan. I'm ok either way.

In D143546#4116196, @HazardyKnusperkeks wrote:

In D143546#4114341, @owenpan wrote:

In D143546#4113721, @rymiel wrote:

In D143546#4112077, @owenpan wrote:

As this one is an invalid-code-generation bug, I wanted it fixed ASAP.

Do you intend to backport it to the 16 release branch then?

I don't know but will go with whatever you, @MyDeveloperDay and @HazardyKnusperkeks prefer.

I'd vote in favor, letting a code breaking bug knowingly in the most current (or to come? I don't follow that.) version would be let's say not nice.

In D143546#4113721, @rymiel wrote:

In D143546#4112077, @owenpan wrote:

As this one is an invalid-code-generation bug, I wanted it fixed ASAP.

Do you intend to backport it to the 16 release branch then?

In D143546#4113730, @rsmith wrote:

I wonder if this can be fixed more generally by using TokenConcatenation::AvoidConcat to determine whether clang-format should require a space between two tokens.

In D141098#4111890, @sunho wrote:

I think it makes more sense to use stable version of clang-format for that purpose but I'm not in a postion to decide as I don't work with format. Can you change the script to use latest clang-format for format codebase and revert this patch in the meantime?

In D143546#4111827, @rymiel wrote:

I've been sniped! (but only because i spent hours discussing with people if the error is pedantically valid, which it turns out it is)

In D141098#4111830, @sunho wrote:

Wouldn't that mean llvm contributors have to format their code using clang-format built from top-of-tree source code before submitting the patch?

Only if the patch is to clang-format source.

In D141098#4108286, @sunho wrote:

I think buildbot is "formatting" the patch using clang-format-15. The tests of clang-format would be done with latest code.

Why don't we fix the buildbot instead? Why does it use a pre-16 version to test 16 source?