Skip to content

#defines split into lines, adds \ and spaces not needed #53180

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

Open
mplexor opened this issue Jan 13, 2022 · 11 comments
Open

#defines split into lines, adds \ and spaces not needed #53180

mplexor opened this issue Jan 13, 2022 · 11 comments
Labels
bug Indicates an unexpected problem or unintended behavior clang-format

Comments

@mplexor
Copy link

mplexor commented Jan 13, 2022

format file:

---
#Language:        Cpp
BasedOnStyle:  LLVM

original single line:

#define grid float3 { 128, 128, 128 }

formats into:

#define grid                         \
 float3 { 128, 128, 128 }
@mplexor mplexor changed the title short #defines split into lines, adds spaces not needed #defines split into lines, adds spaces not needed Jan 13, 2022
@mkurdej
Copy link
Member

mkurdej commented Jan 13, 2022

Please provide the original code and the formatted result. Use markdown to make things easier to read.

@llvmbot
Copy link
Member

llvmbot commented Jan 13, 2022

@llvm/issue-subscribers-clang-format

@mplexor mplexor changed the title #defines split into lines, adds spaces not needed #defines split into lines, adds \ and spaces not needed Jan 13, 2022
@mplexor
Copy link
Author

mplexor commented Jan 13, 2022

updated above. Thanks.

@mkurdej
Copy link
Member

mkurdej commented Jan 13, 2022

Thanks!

Note to self: It's apparently similar to #49164, but the fix (in review https://reviews.llvm.org/D116859) doesn't fix this bug. Also, it doesn't depend on ColumnLimit.

@mkurdej mkurdej added the bug Indicates an unexpected problem or unintended behavior label Jan 13, 2022
@llvmbot
Copy link
Member

llvmbot commented Jan 13, 2022

@llvm/issue-subscribers-bug

@mkurdej
Copy link
Member

mkurdej commented Jan 13, 2022

Minimal repro:

#define A {}

gets formatted to

#define A                                                                      \
  {}

@mplexor
Copy link
Author

mplexor commented Jan 13, 2022

understood.
I have set column limit unbound (set 0), since all variable or trailing comment alignment features I found not useful.

Reason is purely due to use of variable spacing fonts like Verdana (not fixed spacing).
Fixed spacing based alignment will not work for those who use different font, alignment should be dynamically based on user's font in use (should be implemented part of editor rendering).

@mkurdej
Copy link
Member

mkurdej commented Jan 13, 2022

understood. I have set column limit unbound (set 0), since all variable or trailing comment alignment features I found not useful.

Reason is purely due to use of variable spacing fonts like Verdana (not fixed spacing). Fixed spacing based alignment will not work for those who use different font, alignment should be dynamically based on user's font in use (should be implemented part of editor rendering).

Not sure I understood what you mean here. clang-format has certainly nothing to do with the font :D.

@mplexor mplexor closed this as completed Feb 12, 2022
@EugeneZelenko
Copy link
Contributor

Please give developers time to address problems.

@LiamTyler
Copy link

And news on this? I stumbled upon this same thing today

LiamTyler added a commit to LiamTyler/Progression that referenced this issue Dec 2, 2023
I spent a decent chunk of time trying to get clang-format to give the format I want exactly. Ran into many issues with that. Probably some of those are solvable, but I think some of that are not currently. Either way, it's too many issues to actually use regularly. I chose to do 1 pass over the codebase with it anyways (fixing up a couple things here and there) despite this, because my desired format has slowly changed over the years. This code base contains several different formats as my style has changed, and it feels very messy and all over the place. Running this once, even if it has some issues, at least increases the total consistency. I did try to clean up some egregious parts though, like the #pragma issue

Big changes from my regular format:
- I like some 1-line functions, particularly when you are declaring many of them back to back and they are simple returns. But the clang-format file changed *every* function to be 1 line, if they were under the column width limit.
- namespaces no longer ever change indentation

Issues with clang format:
- Does not consistently put the opening brace '{' on a newline. Ex: multi-line array assignments
- Sometimes you have 0 function arguments on the first line (if they all fit onto the 2nd line)
- No way to specify the number of newlines between functions
- I like assignment alignment when the types are the same or similar length. But I can't find a setting to keep the alignment as-is. It's either you align everything (even if it moves the values 80 characters), or none of it
- There seems to be a long standing bug where it will add newlines to macros if they use braces at all llvm/llvm-project#53180
- defines, like "#pragma omp parallel for" are automatically de-indented
- If you have an array of brace initialized structs, it messes up comments before the first line, and the first arg of the first struct is missing indendation....
@yous
Copy link

yous commented May 7, 2024

clang-format 18 partially fixes this, but still have some issues:

test.c:

#define foo bar { 1, 2 }
#define baz { 3, 4 }
$ clang-format --version
clang-format version 17.0.6 (https://github.com/ssciwr/clang-format-wheel 00700f257e3601a6ef0e714031c1dcef8077633f)
$ clang-format --style='{ColumnLimit: 0}' test.c
#define foo \
  bar { 1, 2 }
#define baz \
  { 3, 4 }
$ clang-format --version
clang-format version 18.1.5
$ clang-format --style='{ColumnLimit: 0}' test.c
#define foo \
  bar { 1, 2 }
#define baz {3, 4}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior clang-format
Projects
None yet
Development

No branches or pull requests

6 participants