Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

clang-format: AlignAfterOpenBracket: AlwaysBreak not having an effect on braced blocks results in annoying formatting of requires expressions in concept definitions #57108

Closed
griwes opened this issue Aug 12, 2022 · 2 comments
Assignees

Comments

@griwes
Copy link

griwes commented Aug 12, 2022

Consider the following snippet:

template<typename T>
concept foo = true;

template<typename T>
concept bar = foo<T> && requires (T t) { { t.some_very_long_function_call_goes_here(1,2,3,4,5)} -> foo; };

I have a config with a 110 column limit, 4 space indent, and AlignAfterOpenBracket: AlwaysBreak. The result of formatting the above should be something like this:

template<typename T>
concept foo = true;

template<typename T>
concept bar = foo<T> && requires(T t) {
    { t.some_very_long_function_call_goes_here(1, 2, 3, 4, 5) } -> foo;
};

However, clang-format-15 turns it into the following:

template<typename T>
concept foo = true;

template<typename T>
concept bar = foo<T> && requires(T t) {
                            {
                                t.some_very_long_function_call_goes_here(1, 2, 3, 4, 5)
                                } -> foo;
                        };

This is broken in two ways, but, surprisingly, the wild placement of the braces doesn't actually annoy me that much; arguably, there is a sane coding style to find there if the closing brace was aligned with the opening brace around the atomic constraint.

The alignment of the whole block, though, is really bad. clang-format also ignores this config flag when formatting lambdas in subexpressions, but that is a much more minor annoyance IMHO; yanking a lambda out into a variable when we get to this behavior arguably results in cleaner code. The same cannot, however, be said about concepts, because concepts can only be placed at a namespace scope, so factoring a requires expression out pollutes that namespace with symbols that aren't a part of the interface.

And yes, I am aware that the docs do say that the option doesn't apply to braces, but that is in fact the bug here. Though if there's desire to not have this option change the behavior of braced blocks like this, I'd be happy to get a separate option to control this.

I would be happy to make an attempt at a solution for this, but quick searching through the codebase for the use of the config flag didn't help me with understanding of what would need to happen for this option to become correctly accepted for these cases, so if someone is able to give me a pointers on where to start, maybe I can fix this myself.

@llvmbot
Copy link
Collaborator

llvmbot commented Aug 12, 2022

@llvm/issue-subscribers-clang-format

@rymiel rymiel self-assigned this Sep 26, 2022
@rymiel
Copy link
Member

rymiel commented Sep 26, 2022

Patch for one part of this issue (misaligment of the closing brace compared to the opening brace of the compound requirement): https://reviews.llvm.org/D134626

The other part (indentation of the whole requires expression) is arguably a duplicate of #56283

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants