Skip to content

[clang-format] Incorrect formatting for annotated constrained functions #69325

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

Closed
usx95 opened this issue Oct 17, 2023 · 10 comments · Fixed by #70602
Closed

[clang-format] Incorrect formatting for annotated constrained functions #69325

usx95 opened this issue Oct 17, 2023 · 10 comments · Fixed by #70602
Assignees

Comments

@usx95
Copy link
Contributor

usx95 commented Oct 17, 2023

clang-format's output:

#define ANNOTATE(A) [[clang::annotate(A)]]
template <typename Message>
    requires(true)
ANNOTATE(
    "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message"
    "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message"
    "message") void foo(){};

Expected formatting:

#define ANNOTATE(A) [[clang::annotate(A)]]
template <typename Message>
    requires(true)
ANNOTATE(
    "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message"
    "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message"
    "message") 
void foo(){};

removing the requires clause fixes the formatting (moved void foo() to the next line).

@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2023

@llvm/issue-subscribers-clang-format

Author: Utkarsh Saxena (usx95)

clang-format's output: ``` #define ANNOTATE(A) [[clang::annotate(A)]] template <typename Message> requires(true) ANNOTATE( "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message" "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message" "message") void foo(){}; ``` Expected formatting: ``` #define ANNOTATE(A) [[clang::annotate(A)]] template <typename Message> requires(true) ANNOTATE( "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message" "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message" "message") void foo(){}; ```

removing the requires clause fixes the formatting (moved void foo() to the next line).

@llvmbot
Copy link
Member

llvmbot commented Oct 17, 2023

@llvm/issue-subscribers-c-20

Author: Utkarsh Saxena (usx95)

clang-format's output: ``` #define ANNOTATE(A) [[clang::annotate(A)]] template <typename Message> requires(true) ANNOTATE( "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message" "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message" "message") void foo(){}; ``` Expected formatting: ``` #define ANNOTATE(A) [[clang::annotate(A)]] template <typename Message> requires(true) ANNOTATE( "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message" "Loooooooooooooooooooooooooooooooooooooooooooooooooooong message" "message") void foo(){}; ```

removing the requires clause fixes the formatting (moved void foo() to the next line).

@usx95
Copy link
Contributor Author

usx95 commented Oct 17, 2023

+CC @rymiel @HazardyKnusperkeks

@HazardyKnusperkeks
Copy link
Contributor

I'll take a look what's the difference. In the meantime maybe https://clang.llvm.org/docs/ClangFormatStyleOptions.html#macros could help you.

@owenca
Copy link
Contributor

owenca commented Oct 20, 2023

The clang-format settings below would work:

BreakAfterAttributes: Always
Macros:
- ANNOTATE(x)=[[a(x)]]

@usx95
Copy link
Contributor Author

usx95 commented Oct 26, 2023

I think the problem is that we do not want to "always" have a break (for example, when the annotation is small enough).

The problem is that the existence of a requires clause removes the line break, which looks unexpected.

@HazardyKnusperkeks HazardyKnusperkeks self-assigned this Oct 26, 2023
@HazardyKnusperkeks
Copy link
Contributor

I'd say we have two issues here. One is that it is handled differently with the requires clause. But the bigger issue would be that foo is misannotated in both cases:

 M=0 C=1 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=void L=120 PPK=2 FakeLParens= FakeRParens=0 II=0x559e1e3dbcb0 Text='void'
 M=0 C=0 T=TrailingAnnotation S=1 F=0 B=0 BK=0 P=23 Name=identifier L=124 PPK=2 FakeLParens= FakeRParens=0 II=0x559e1e4205e0 Text='foo'

I'll try to fix the second issue, and then we'll see if the first is still around.

@HazardyKnusperkeks
Copy link
Contributor

If you declare your ANNOTATE as AttributeMacros both variants have no line break before void, but after that. ;)

@owenca
Copy link
Contributor

owenca commented Oct 29, 2023

I think the problem is that we do not want to "always" have a break (for example, when the annotation is small enough).

Then setting it to Leave would work, right?

@HazardyKnusperkeks
Copy link
Contributor

HazardyKnusperkeks commented Oct 29, 2023

So I actually corrected the first issue. The second issue will be tracked in #70603.

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

Successfully merging a pull request may close this issue.

4 participants