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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
