clang-format misplaces * in declaration of pointer to struct

[fanf2]

Our code in BIND is auto-formatted with clang-format using a .clang-format configuration that includes PointerAlignment: Right and PointerBindsToType: false

When I run the following on the command line:

$ echo 'struct { int foo; } *foop;' | clang-format
struct {
	int foo;
} * foop;

I get a space before foop, where I expected the last line to be } *foop;

[llvmbot]

@llvm/issue-subscribers-clang-format

[mydeveloperday]

PointerBindsToType I'm not sure thats a current option

Having said that I kind of expect it to work with just PointerAlignment, (but I see it doesn't)

struct {
  int foo;
} * foop;

BasedOnStyle: LLVM
PointerAlignment: Right

[llvmbot]

@llvm/issue-subscribers-bug

[llvmbot]

@llvm/issue-subscribers-good-first-issue

[jackhong12]

The problem is that determineStarAmpUsage categorizes the star token as a binary operator.
In my opinion, we can remove the tok::r_brace condition because star tokens will not be binary operators when they are behind right brackets in C and C++.

    if (PrevToken->Tok.isLiteral() ||
        PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
                           tok::kw_false, tok::r_brace)) {
      return TT_BinaryOperator;
    }

By the way, I want to add new test cases in FormatTest.cpp. But I am a newbie in LLVM and don't know how to run unit tests. Could somebody tell me? Thanks.

[mydeveloperday]

I build on windows (in a visual studio shell), and I do this (one time)

mkdir build
cd build
export CC=cl.exe
export CXX=cl.exe
c:/Program\ Files/CMake/bin/cmake.exe ../llvm-project/llvm -G "Ninja" \
-DLLVM_BUILD_TESTS=ON \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_EXPORT_COMPILE_COMMANDS=ON \
-DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra"

Then I do this in the build directory

ninja clang-format
ninja FormatTests

./tools/clang/unittests/Format/FormatTests.exe

[mydeveloperday]

we can remove the tok::r_brace condition because star tokens

My assumption would be that this will break one of the existing unit tests... you won't know until you try..

are you saying ... } * .... won't be a binary operator? that is true but this is &,&& and * so you have to consider

} & and } && and that can occur.. you will soon discover this use case

 template <class T,
           class = typename ::std::enable_if<
              ::std::is_array<T>{} && ::std::is_array<T>{}>::type>

But you may be able to handle that case by something like:

     if (PrevToken->Tok.isLiteral() ||
         PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::kw_true,
-                           tok::kw_false, tok::r_brace))
+                           tok::kw_false))
+      return TT_BinaryOperator;
+
+    if (PrevToken->is(tok::r_brace) && Tok.isOneOf(tok::amp, tok::ampamp))
       return TT_BinaryOperator;

     const FormatToken *NextNonParen = NextToken;

[jackhong12]

@mydeveloperday You are right. Thanks a lot.

[jackhong12]

Revision: https://reviews.llvm.org/D127873