[clang] [clang-forma] Support `PointerAlignment` for pointers to members (PR #86253)

kadir çetinkaya via cfe-commits cfe-commits at lists.llvm.org
Tue Aug 20 02:30:10 PDT 2024


kadircet wrote:

Hi folks, I am a little confused about the behavior introduced by this patch.

I was expecting `Foo::*` in `Type Foo::* name` to be treated as a single entity and whole thing to be aligned with `name` or spaced away from it, Rather than splitting `*` . To add some more from the grammar, Pointers, and pointers-to-members are grammatically different:
- The former is introduced by appending `*` to another declarator, see https://eel.is/c++draft/dcl.ptr.
- Whereas the latter is introduced by appending `nested-name-spec*`, see https://eel.is/c++draft/dcl.mptr.

So I think there is value in keeping `nested-name-spec` and `*` together and moving the whole `nested-name-spec::*` block, rather than just moving `*` left or right. e.g, instead of `int Foo:: *member;` we should prefer:
- `int Foo::*member;` or -- pointer alignment right
- `int Foo::* member;` -- pointer alignment left

Looking at the bug https://github.com/llvm/llvm-project/issues/85761, I can't see any strong indications towards just moving `*` around. Moreover user is actually talking about space between `::*` and `name`, hence I think they were also intending for `::*` to stay together.

Changing behavior to keep `nested-name-specifier` and `*` together also ensures this is a no-op when pointer-alignment is right.

Were there any strong reasons to perform a split after `::`?

---

Second surprising change is around method pointers, we currently have [logic](https://github.com/llvm/llvm-project/blame/a6d81cdf896d901e0f5e672b9a3eccc4ae8759ce/clang/lib/Format/TokenAnnotator.cpp#L4597) to never separate pointer and name in function-pointers like `void (*foo)();`. 
This applied to method-pointers as well, e.g. `void (Foo::*f)();`. After this change we introduced a different behavior for this case when we have a method pointer instead of function pointer.

Was this change intentional? If not can we restore the unified behavior between handling of method-pointers and function-pointers? This issue is also reported by https://github.com/llvm/llvm-project/issues/100841, cc @vogelsgesang 

P.S. I am happy to do these changes myself, I just want to make sure we agree on the desired behaviors here.

https://github.com/llvm/llvm-project/pull/86253


More information about the cfe-commits mailing list