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

Incorrect clang-format pointer alignment when variables are declared in conditionals #60146

Closed
JohnC32 opened this issue Jan 19, 2023 · 10 comments · Fixed by #109370
Closed

Incorrect clang-format pointer alignment when variables are declared in conditionals #60146

JohnC32 opened this issue Jan 19, 2023 · 10 comments · Fixed by #109370
Assignees

Comments

@JohnC32
Copy link

JohnC32 commented Jan 19, 2023

Given:

void foo1(const T1* p1) {
    if (T2* v; check(&v, p1)) {
        goo(v);
    }
}

and _clang-format:

---
BasedOnStyle: Google
IndentWidth: 4
# Google style default is PointerAlignment: Left,
PointerAlignment: Left
...

clang-format 15 yields the following. Notice the pointer alignment is no longer left, despite
asking for it to be left.

void foo1(const T1* p1) {
    if (T2 * v; check(&v, p1)) {
        goo(v);
    }
}

In this case, we scoped the variable declaration in the if conditional. If we move it out of the
conditional, pointer alignment is as expected:

void foo2(const T1* p1) {
    T2* v;
    if (check(&v, p1)) {
        goo(v);
    }
}
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 19, 2023

@llvm/issue-subscribers-clang-format

@jeremyong
Copy link

I'm seeing this also with an even simpler repro:

for (int i = 0; Foo& foo : foos)

turns into

for (int i = 0; Foo & foo : foos)

even with PointerAlignment: Left. I am assuming that in the presence of scoped declaration statements, subsequent & operators are parsed as bitwise-and operators.

@swalkner
Copy link

swalkner commented Sep 18, 2024

I can confirm this behaviour and would like to gently bump this report.

if (MyClass * obj{GetMyClass()})

But - for some reason - auto and primitive-types seem to work as expected:

if (auto* obj{GetMyClass()})
if (int* obj{GetInt()})

I have reproduced that in clang-format:

  • 17.0.3 (packaged with VS)
  • 18.1.3 (and other versions via clang-format-configurator.site)

The only config option I changed:

PointerAlignment: Left

@CDAlexanderW
Copy link

Is there a chance for a fix for this?
@owenca

@owenca
Copy link
Contributor

owenca commented Sep 19, 2024

We have the TypeNames option for user-defined types when clang-format can't tell if an identifier before */&/&& is a type or variable:

$ clang-format -version
clang-format version 19.1.0
$ cat foo.cpp
if (T2 *v; check(&v, p1))
  ;
for (int i = 0; Foo &foo : foos)
  ;
if (MyClass *obj{GetMyClass()})
  ;
$ cat .clang-format
TypeNames: [T2, Foo, MyClass]
$ clang-format foo.cpp | diff foo.cpp -
$ 

@CDAlexanderW
Copy link

We have the TypeNames option for user-defined types when clang-format can't tell if an identifier before */&/&& is a type or variable:

$ clang-format -version
clang-format version 19.1.0
$ cat foo.cpp
if (T2 *v; check(&v, p1))
  ;
for (int i = 0; Foo &foo : foos)
  ;
if (MyClass *obj{GetMyClass()})
  ;
$ cat .clang-format
TypeNames: [T2, Foo, MyClass]
$ clang-format foo.cpp | diff foo.cpp -
$ 

So just to be sure. Technically we dont have any better option besides providing this additional TypeNames? For very large code bases with millions of types (although not all used in such cases but still a lot) it may not be possible to provide all the types. For this we mas have to live with it, correct?

Thx!

@owenca
Copy link
Contributor

owenca commented Sep 20, 2024

Technically we dont have any better option besides providing this additional TypeNames?

In general, TypeNames can be used if there are no better alternatives. (See https://reviews.llvm.org/D155273 for some background.) Nevertheless, I'll see if something similar to #109229 can be done here.

@owenca
Copy link
Contributor

owenca commented Sep 20, 2024

I'm seeing this also with an even simpler repro:

for (int i = 0; Foo& foo : foos)

turns into

for (int i = 0; Foo & foo : foos)

even with PointerAlignment: Left. I am assuming that in the presence of scoped declaration statements, subsequent & operators are parsed as bitwise-and operators.

This is related but different. I have moved it to #109358.

@owenca
Copy link
Contributor

owenca commented Sep 20, 2024

I can confirm this behaviour and would like to gently bump this report.

if (MyClass * obj{GetMyClass()})

But - for some reason - auto and primitive-types seem to work as expected:

if (auto* obj{GetMyClass()})
if (int* obj{GetInt()})

I have reproduced that in clang-format:

  • 17.0.3 (packaged with VS)
  • 18.1.3 (and other versions via clang-format-configurator.site)

The only config option I changed:

PointerAlignment: Left

This is different as there is no semicolon. Please split it to a new issue or use the TypeNames option.

@swalkner
Copy link

Thanks so much for your reply. I created #109371

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.

7 participants