Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clang/lib/Format/TokenAnnotator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3802,7 +3802,7 @@ static bool isFunctionDeclarationName(const LangOptions &LangOpts,
const auto *Prev = Current.getPreviousNonComment();
assert(Prev);

if (Prev->is(tok::coloncolon))
if (Prev->is(tok::coloncolon) && Prev->hasWhitespaceBefore())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But SomeAPI ::operator()(); has space before and still gets miss annotated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But SomeAPI ::operator()(); has space before and still gets miss annotated.

Not necessarily. See e.g.

Tokens = annotate("friend ostream& ::operator<<(ostream& lhs, foo& rhs);");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But I feed exactly that string and it got the TT_FunctionDeclarationName.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, because it is a function declaration there. So we annotate this kind of constructs as follows:

Foo::operator()();  // function call
Bar ::operator<<(); // function declaration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No it is not. It's the same, just with a space in the input:

echo "SomeAPI::operator()();             
SomeAPI ::operator()();" | clang-format --debug-only=format-token-annotator
AnnotatedTokens(L=0, P=0, T=6, C=0):
 I=0 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=identifier N=0 L=7 PPK=2 FakeLParens= FakeRParens=0 II=0x558d281139e0 Text='SomeAPI'
 I=0 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=coloncolon N=0 L=9 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='::'
 I=0 M=0 C=1 T=Unknown S=0 F=0 B=0 BK=0 P=520 Name=operator N=0 L=17 PPK=2 FakeLParens= FakeRParens=0 II=0x558d281067c8 Text='operator'
 I=0 M=0 C=0 T=OverloadedOperator S=0 F=0 B=0 BK=0 P=23 Name=l_paren N=0 L=18 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 I=0 M=0 C=0 T=OverloadedOperator S=0 F=0 B=0 BK=0 P=140 Name=r_paren N=1 L=19 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 I=0 M=0 C=0 T=OverloadedOperatorLParen S=0 F=0 B=0 BK=0 P=23 Name=l_paren N=0 L=20 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 I=0 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=140 Name=r_paren N=1 L=21 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 I=0 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi N=0 L=22 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'
----
AnnotatedTokens(L=0, P=0, T=6, C=0):
 I=0 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=identifier N=0 L=7 PPK=2 FakeLParens= FakeRParens=0 II=0x558d281139e0 Text='SomeAPI'
 I=0 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=23 Name=coloncolon N=0 L=10 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='::'
 I=0 M=0 C=1 T=FunctionDeclarationName S=0 F=0 B=0 BK=0 P=520 Name=operator N=0 L=18 PPK=2 FakeLParens= FakeRParens=0 II=0x558d281067c8 Text='operator'
 I=0 M=0 C=0 T=OverloadedOperator S=0 F=0 B=0 BK=0 P=23 Name=l_paren N=0 L=19 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 I=0 M=0 C=0 T=OverloadedOperator S=0 F=0 B=0 BK=0 P=140 Name=r_paren N=1 L=20 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 I=0 M=0 C=0 T=OverloadedOperatorLParen S=0 F=0 B=0 BK=0 P=23 Name=l_paren N=0 L=21 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text='('
 I=0 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=140 Name=r_paren N=1 L=22 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=')'
 I=0 M=0 C=0 T=Unknown S=0 F=0 B=0 BK=0 P=23 Name=semi N=0 L=23 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=';'
----
AnnotatedTokens(L=0, P=0, T=6, C=0):
 I=0 M=0 C=0 T=Unknown S=1 F=0 B=0 BK=0 P=0 Name=eof N=0 L=0 PPK=2 FakeLParens= FakeRParens=0 II=0x0 Text=''
----
SomeAPI::operator()();
SomeAPI ::operator()();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

::operator preceded by a tok::identifier can very well be a function decl. See e.g. Out ::operator<<( in here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it can be, that was never the point of any discussion (on my side). But isn't this PR not about to not mark a function call to ::operator as declaration? And I just show, that the same code, just with a space before :: is annotated differently.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's why I said we should annotate them as intended by the user.

So we annotate this kind of constructs as follows:

Foo::operator()();  // function call
Bar ::operator<<(); // function declaration

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why you always bring the up function declaration.
Before your patch:

  auto Tokens = annotate("SomeAPI::operator()();\n"
                         "SomeAPI ::operator()();");
  EXPECT_TOKEN(Tokens[2], tok::kw_operator, TT_FunctionDeclarationName);
  EXPECT_TOKEN(Tokens[10], tok::kw_operator, TT_FunctionDeclarationName);

After your patch:

  auto Tokens = annotate("SomeAPI::operator()();\n"
                         "SomeAPI ::operator()();");
  EXPECT_TOKEN(Tokens[2], tok::kw_operator, TT_Unknown);
  EXPECT_TOKEN(Tokens[10], tok::kw_operator, TT_FunctionDeclarationName);

It only correctly annotates the first line, with no space before the ::.

That's all I say.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I give up on trying to explain further as it's a moot point with #164048.

Prev = Prev->Previous;

if (!Prev)
Expand Down
5 changes: 5 additions & 0 deletions clang/unittests/Format/TokenAnnotatorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1129,6 +1129,11 @@ TEST_F(TokenAnnotatorTest, UnderstandsOverloadedOperators) {
ASSERT_EQ(Tokens.size(), 7u) << Tokens;
// Not TT_FunctionDeclarationName.
EXPECT_TOKEN(Tokens[3], tok::kw_operator, TT_Unknown);

Tokens = annotate("SomeAPI::operator()();");
ASSERT_EQ(Tokens.size(), 9u) << Tokens;
// Not TT_FunctionDeclarationName.
EXPECT_TOKEN(Tokens[2], tok::kw_operator, TT_Unknown);
}

TEST_F(TokenAnnotatorTest, OverloadedOperatorInTemplate) {
Expand Down