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 adds new line between semicolon and body-less loop #61708

Closed
Jorengarenar opened this issue Mar 26, 2023 · 13 comments · Fixed by #70768
Closed

clang-format adds new line between semicolon and body-less loop #61708

Jorengarenar opened this issue Mar 26, 2023 · 13 comments · Fixed by #70768

Comments

@Jorengarenar
Copy link

I'm gonna borrow example from this SO question (and here's a similar one):

I want to configure clang-format that it leaves single-line while-statement without adding a line break for the trailing semicolon.
For example, clang-format should just leave a "one-liner" as it is:

while (checkWaitCondition() != true);

But unfortunately it adds by default a line break (plus an indentation):

while (checkWaitCondition() != true)
  ;
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 26, 2023

@llvm/issue-subscribers-clang-format

@gedare
Copy link
Contributor

gedare commented Jun 12, 2023

Steps to reproduce:

$ echo "while(1);" | clang-format -style=LLVM
while (1)
  ;

I have this behavior on 16.0.0 and 17.0.0. I do not have an older version to test or bisect setup at the moment.

@rymiel
Copy link
Member

rymiel commented Jun 12, 2023

I do not have an older version to test or bisect setup at the moment.

That's fine, this is clang-format's behavior since its inception (i think) and definitely intended. Question is, do we want this to be configurable

@gedare
Copy link
Contributor

gedare commented Jun 14, 2023

I do not have an older version to test or bisect setup at the moment.

That's fine, this is clang-format's behavior since its inception (i think) and definitely intended. Question is, do we want this to be configurable

I would like to make this configurable. Any suggestions are welcome, otherwise I may try to bring a proposal. Maybe BreakBeforeEmptyLoopBody or AllowEmptyLoopsOnASingleLine

@owenca
Copy link
Contributor

owenca commented Aug 6, 2023

Steps to reproduce:

$ echo "while(1);" | clang-format -style=LLVM
while (1)
  ;

The default value of AllowShortLoopsOnASingleLine for LLVM is false. To demonstrate that the loop is not merged despite that AllowShortLoopsOnASingleLine is changed to true:

 $ echo "while(1);" | clang-format -style='{AllowShortLoopsOnASingleLine: true}'
while (1)
  ;

@owenca
Copy link
Contributor

owenca commented Aug 6, 2023

owenca added a commit to owenca/llvm-project that referenced this issue Oct 31, 2023
A for/while loop with only a semicolon as its body should be treated as an
empty loop.

Fixes llvm#61708.
@HazardyKnusperkeks
Copy link
Contributor

I do not have an older version to test or bisect setup at the moment.

That's fine, this is clang-format's behavior since its inception (i think) and definitely intended. Question is, do we want this to be configurable

I'd say this should not be formatted on a single line. I'd change AllowShortLoopsOnASingleLine to Never, NonEmpty (this is the old true) and Always, or something like that.

@owenca
Copy link
Contributor

owenca commented Oct 31, 2023

See the discussion from here and onward.

owenca added a commit that referenced this issue Nov 2, 2023
A for/while loop with only a semicolon as its body should be treated as
an empty loop.

Fixes #61708.
@prj-
Copy link

prj- commented Nov 27, 2023

@owenca, @HazardyKnusperkeks, sorry to bump this old thread, but is there a way to revert to the "old" behavior, i.e., having the semi-colon on a single line? It seems https://reviews.llvm.org/D154550#4526386 never got merged, so I cannot use NonNullStatement.

@owenca
Copy link
Contributor

owenca commented Nov 27, 2023

is there a way to revert to the "old" behavior, i.e., having the semi-colon on a single line?

Yes, by setting AllowShortLoopsOnASingleLine to false.

@prj-
Copy link

prj- commented Nov 27, 2023

This, however, introduces a very large number of other white-space changes. Is something like AllowEmptyLoopsOnASingleLine planned at all?

@owenca
Copy link
Contributor

owenca commented Nov 27, 2023

I don't think so, if you mean:

while (i > 0) --i;
while (0)
  ;
while (j > 0) { --j; }
while (1)
{}

Empty loops are short loops and should follow the setting of AllowShortLoopsOnASingleLine.

@prj-
Copy link

prj- commented Nov 27, 2023

OK, thanks, no problem.

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.

8 participants