LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 40418 - AlignConsecutiveDeclarations fails with attibutes
Summary: AlignConsecutiveDeclarations fails with attibutes
Status: CONFIRMED
Alias: None
Product: clang
Classification: Unclassified
Component: Formatter (show other bugs)
Version: 7.0
Hardware: All All
: P normal
Assignee: Unassigned Clang Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-01-22 16:58 PST by Roland Schulz
Modified: 2019-03-13 14:11 PDT (History)
4 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Roland Schulz 2019-01-22 16:58:22 PST
clang-format produces:

void f(int  extremlylongparameternamexxxxxxxx1,
       long extremlylongparameternamexxxxxxxx2,
       int[[]] extremlylongparameternamexxxxxxxx3,
       int __attribute__(()) extremlylongparameternamexxxxxxxx4) {
  int           a;
  unsigned long b;
  int[[]] c;
  int __attribute__(()) d;
}

Given configuration:
AlignConsecutiveDeclarations: true

It fails to align both the parameters and variables declarations with attributes. Other lines work fine. But those with attributes are ignored.
Comment 1 MyDeveloperDay 2019-03-13 12:10:17 PDT
clang-format doesn't see the ( and [ of the attributes as being part of what can determine the space between the type and the name

void f(int extremlylongparameternamexxxxxxxx1, long extremlylongparameternamexxxxxxxx2,
       int[[]] extremlylongparameternamexxxxxxxx3,
       int __attribute__(()) extremlylongparameternamexxxxxxxx4) {
  int           a;
  unsigned long b;
  int __attribute__(()) d;
  int[[]] c;
}

void f(int extremlylongparameternamexxxxxxxx1, long extremlylongparameternamexxxxxxxx2,
       int____ extremlylongparameternamexxxxxxxx3,
       int __attribute______ extremlylongparameternamexxxxxxxx4) {
  int           a;
  unsigned long b;
  int __attribute______ d;
  int____               c;
}

void f(int extremlylongparameternamexxxxxxxx1, long extremlylongparameternamexxxxxxxx2,
       int [[clang::nodiscard]] extremlylongparameternamexxxxxxxx3,
       int __attribute__((clang::nodiscard)) extremlylongparameternamexxxxxxxx4) {
  int           a;
  unsigned long b;
  int __attribute__((clang::nodiscard)) d;
  int [[clang::nodiscard]] c;
}


When we run this with the --debug option if we look at this line

int __attribute______ d;

its represented as

AnnotatedTokens(L=1):
 M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=int L=3 PPK=2 FakeLParens= FakeRParens=0 II=0x1afa8e48470 Text='int'
 M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=220 Name=identifier L=21 PPK=2 FakeLParens= FakeRParens=0 II=0x1afa8e49f70 Text='__attribute______'
 M=0 C=1 T=StartOfName S=1 B=0 BK=0 P=130 Name=identifier L=23 PPK=2 FakeLParens= FakeRParens=0 II=0x1afa8e49ed0 Text='d'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=semi L=24 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'
----

but this line

int __attribute__((clang::nodiscard)) d;

it more complex,

AnnotatedTokens(L=1):
 M=0 C=0 T=Unknown S=1 B=0 BK=0 P=0 Name=int L=3 PPK=2 FakeLParens= FakeRParens=0 II=0x1afa8e48470 Text='int'
 M=0 C=1 T=Unknown S=1 B=0 BK=0 P=23 Name=__attribute L=17 PPK=2 FakeLParens= FakeRParens=0 II=0x1afa8e46ee8 Text='__attribute__'
 M=0 C=0 T=AttributeParen S=0 B=0 BK=0 P=23 Name=l_paren L=18 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=59 Name=l_paren L=19 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=79 Name=identifier L=24 PPK=2 FakeLParens= FakeRParens=0 II=0x1afa8e49fa0 Text='clang'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=63 Name=coloncolon L=26 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='::'
 M=0 C=1 T=Unknown S=0 B=0 BK=0 P=560 Name=identifier L=35 PPK=2 FakeLParens= FakeRParens=0 II=0x1afa8e49fd8 Text='nodiscard'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=63 Name=r_paren L=36 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 M=0 C=0 T=AttributeParen S=0 B=0 BK=0 P=43 Name=r_paren L=37 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 M=0 C=1 T=Unknown S=1 B=0 BK=0 P=23 Name=identifier L=39 PPK=2 FakeLParens= FakeRParens=0 II=0x1afa8e49ed0 Text='d'
 M=0 C=0 T=Unknown S=0 B=0 BK=0 P=23 Name=semi L=40 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'
----

In the code, I suspect it needs to be able to handle other token types(l_parent,identifier,coloncolon,r_paren)

void WhitespaceManager::alignConsecutiveDeclarations() {
  if (!Style.AlignConsecutiveDeclarations)
    return;

  // FIXME: Currently we don't handle properly the PointerAlignment: Right
  // The * and & are not aligned and are left dangling. Something has to be done
  // about it, but it raises the question of alignment of code like:
  //   const char* const* v1;
  //   float const* v2;
  //   SomeVeryLongType const& v3;
  AlignTokens(Style,
              [](Change const &C) {
                // tok::kw_operator is necessary for aligning operator overload
                // definitions.
                return C.Tok->is(TT_StartOfName) ||
                       C.Tok->is(TT_FunctionDeclarationName) ||
                       C.Tok->is(tok::kw_operator);
              },
              Changes, /*StartAt=*/0);
}

One way is to look for specific sequences

        return C.Tok->is(TT_StartOfName) ||
               C.Tok->is(TT_FunctionDeclarationName) ||
               C.Tok->startsSequence(tok::l_paren, tok::l_paren, tok::identifier,
                                  tok::coloncolon, tok::identifier,
                                  tok::r_paren, tok::r_paren) ||
               C.Tok->startsSequence(tok::l_square, tok::l_square, tok::identifier,
                                  tok::coloncolon, tok::identifier,
                                  tok::r_square, tok::r_square) ||
               C.Tok->is(tok::kw_operator);

Which handles some cases but not your empty attribute example of [[]] and __attribute(())

After adding these 2 C.Tok->startsSequence clauses, I get the following

void f(int              extremlylongparameternamexxxxxxxx1,
       long             extremlylongparameternamexxxxxxxx2,
       int              [[a::b]] extremlylongparameternamexxxxxxxx3,
       int __attribute__((a::b)) extremlylongparameternamexxxxxxxx4) {
  int              a;
  unsigned long    b;
  int __attribute__((a::b)) d;
  int              [[a::b]] c;
  int __attribute__((a::b)) e;
  int __attribute__((depreciated)) f;
  int __attribute__((aligned(16))) g;
}

but the following with your example

void f(int  extremlylongparameternamexxxxxxxx1,
       long extremlylongparameternamexxxxxxxx2,
       int[[]] extremlylongparameternamexxxxxxxx3,
       int __attribute__(()) extremlylongparameternamexxxxxxxx4) {
  int           a;
  unsigned long b;
  int __attribute__(()) d;
  int[[]] c;
  int __attribute__(()) e;
  int __attribute__((depreciated)) f;
  int __attribute__((aligned(16))) g;
}

Although they too can be handled by adding

return C.Tok->is(TT_StartOfName) ||
               C.Tok->is(TT_FunctionDeclarationName) ||
               C.Tok->startsSequence(tok::l_paren, tok::l_paren, tok::identifier,
                                  tok::coloncolon, tok::identifier,
                                  tok::r_paren, tok::r_paren) ||
               C.Tok->startsSequence(tok::l_square, tok::l_square, tok::identifier,
                                  tok::coloncolon, tok::identifier,
                                  tok::r_square, tok::r_square) ||
               C.Tok->startsSequence(tok::l_paren, tok::l_paren,
                                  tok::r_paren, tok::r_paren) ||
               C.Tok->startsSequence(tok::l_square, tok::l_square,
                                  tok::r_square, tok::r_square) ||
               C.Tok->is(tok::kw_operator);


void f(int              extremlylongparameternamexxxxxxxx1,
       long             extremlylongparameternamexxxxxxxx2,
       int              [[]] extremlylongparameternamexxxxxxxx3,
       int __attribute__(()) extremlylongparameternamexxxxxxxx4) {
  int              a;
  unsigned long    b;
  int __attribute__(()) d;
  int              [[]] c;
  int __attribute__(()) e;
  int __attribute__((depreciated)) f;
  int __attribute__((aligned(16))) g;
}


Before submitting a patch, I'd like to understand if the your empty example has a real world usecase?

MyDeveloperDay
Comment 2 Roland Schulz 2019-03-13 12:31:55 PDT
The empty argument example was just a result of simplifying the real world use case. Our code-base never has empty attribute lists.
Comment 3 MyDeveloperDay 2019-03-13 14:11:30 PDT
WIP https://reviews.llvm.org/D59332