[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

Submitted as r366267. Thanks!




Comment at: clang/lib/Format/TokenAnnotator.cpp:389
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;

aaron.ballman wrote:
> Clang has a feature flag to enable support for double-square bracket 
> attributes in more than just C++ mode, and this is enabled by default in C2x 
> mode. This check for `isCpp()` makes me suspect we may be doing the wrong 
> thing here.
Good point. I filed https://bugs.llvm.org/show_bug.cgi?id=42645 to revisit this.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366267: [clang-format] Dont detect call to ObjC class 
method as C++11 attribute… (authored by benhamilton, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64632?vs=209777=210182#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632

Files:
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -388,6 +388,10 @@
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;
+// The first square bracket is part of an ObjC array literal
+if (Tok.Previous && Tok.Previous->is(tok::at)) {
+  return false;
+}
 const FormatToken *AttrTok = Tok.Next->Next;
 if (!AttrTok)
   return false;
@@ -400,7 +404,7 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]].
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,12 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake Objective-C method inside array literals
+  // as attributes, even if those method names are also keywords.
+  verifyFormat("@[ [foo bar] ];");
+  verifyFormat("@[ [NSArray class] ];");
+  verifyFormat("@[ [foo enum] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -388,6 +388,10 @@
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;
+// The first square bracket is part of an ObjC array literal
+if (Tok.Previous && Tok.Previous->is(tok::at)) {
+  return false;
+}
 const FormatToken *AttrTok = Tok.Next->Next;
 if (!AttrTok)
   return false;
@@ -400,7 +404,7 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]].
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,12 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake Objective-C method inside array literals
+  // as attributes, even if those method names are also keywords.
+  verifyFormat("@[ [foo bar] ];");
+  verifyFormat("@[ [NSArray class] ];");
+  verifyFormat("@[ [foo enum] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Robbie Gibson via Phabricator via cfe-commits]

rkgibson2 added a comment.

I don't think I have commit access. Could someone land this for me?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Robbie Gibson via Phabricator via cfe-commits]

rkgibson2 updated this revision to Diff 209777.
rkgibson2 added a comment.

Add more tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,12 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake Objective-C method inside array literals
+  // as attributes, even if those method names are also keywords.
+  verifyFormat("@[ [foo bar] ];");
+  verifyFormat("@[ [NSArray class] ];");
+  verifyFormat("@[ [foo enum] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -388,6 +388,10 @@
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;
+// The first square bracket is part of an ObjC array literal
+if (Tok.Previous && Tok.Previous->is(tok::at)) {
+  return false;
+}
 const FormatToken *AttrTok = Tok.Next->Next;
 if (!AttrTok)
   return false;
@@ -400,7 +404,7 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]].
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,12 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake Objective-C method inside array literals
+  // as attributes, even if those method names are also keywords.
+  verifyFormat("@[ [foo bar] ];");
+  verifyFormat("@[ [NSArray class] ];");
+  verifyFormat("@[ [foo enum] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -388,6 +388,10 @@
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;
+// The first square bracket is part of an ObjC array literal
+if (Tok.Previous && Tok.Previous->is(tok::at)) {
+  return false;
+}
 const FormatToken *AttrTok = Tok.Next->Next;
 if (!AttrTok)
   return false;
@@ -400,7 +404,7 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]].
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:7030-7032
+  // Make sure that we do not mistake a call to the Objective-C method named
+  // "class" inside an array literal as attributes.
+  verifyFormat("@[ [NSArray class] ];");

Consider adding another test for a method besides +class.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added a comment.
This revision is now accepted and ready to land.

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Robbie Gibson via Phabricator via cfe-commits]

rkgibson2 updated this revision to Diff 209504.
rkgibson2 added a comment.

Update comment


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,10 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake a call to the Objective-C method named
+  // "class" inside an array literal as attributes.
+  verifyFormat("@[ [NSArray class] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -388,6 +388,10 @@
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;
+// The first square bracket is part of an ObjC array literal
+if (Tok.Previous && Tok.Previous->is(tok::at)) {
+  return false;
+}
 const FormatToken *AttrTok = Tok.Next->Next;
 if (!AttrTok)
   return false;
@@ -400,7 +404,7 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]].
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,10 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake a call to the Objective-C method named
+  // "class" inside an array literal as attributes.
+  verifyFormat("@[ [NSArray class] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -388,6 +388,10 @@
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;
+// The first square bracket is part of an ObjC array literal
+if (Tok.Previous && Tok.Previous->is(tok::at)) {
+  return false;
+}
 const FormatToken *AttrTok = Tok.Next->Next;
 if (!AttrTok)
   return false;
@@ -400,7 +404,7 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]].
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:389
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;

Clang has a feature flag to enable support for double-square bracket attributes 
in more than just C++ mode, and this is enabled by default in C2x mode. This 
check for `isCpp()` makes me suspect we may be doing the wrong thing here.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Robbie Gibson via Phabricator via cfe-commits]

rkgibson2 updated this revision to Diff 209496.
rkgibson2 added a comment.

Respond to comments and change detection method


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,10 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake a call to the Objective-C method named
+  // "class" inside an array literal as attributes.
+  verifyFormat("@[ [NSArray class] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -388,6 +388,10 @@
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;
+// Objective-C array literal and message send.
+if (Tok.Previous && Tok.Previous->is(tok::at)) {
+  return false;
+}
 const FormatToken *AttrTok = Tok.Next->Next;
 if (!AttrTok)
   return false;
@@ -400,7 +404,7 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]].
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,10 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake a call to the Objective-C method named
+  // "class" inside an array literal as attributes.
+  verifyFormat("@[ [NSArray class] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -388,6 +388,10 @@
   bool isCpp11AttributeSpecifier(const FormatToken ) {
 if (!Style.isCpp() || !Tok.startsSequence(tok::l_square, tok::l_square))
   return false;
+// Objective-C array literal and message send.
+if (Tok.Previous && Tok.Previous->is(tok::at)) {
+  return false;
+}
 const FormatToken *AttrTok = Tok.Next->Next;
 if (!AttrTok)
   return false;
@@ -400,7 +404,7 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]].
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Robbie Gibson via Phabricator via cfe-commits]

rkgibson2 marked an inline comment as done.
rkgibson2 added a comment.

In D64632#1582793 , @benhamilton wrote:

> Thanks for the fix. One question: how does the real Clang parser deal with 
> this case? Is it something that's actually ambiguous in the ObjC++ grammar, I 
> wonder?


I am not sure about real Clang. I actually don't know much about how it works. 
I just ran into this bug and wanted to see if it was fixable. My guess is the @ 
sign marks the bracket as an array literal. In clang-format, 
isCpp11AttributeSpecifier is checked before checking whether the previous token 
is @.




Comment at: clang/lib/Format/TokenAnnotator.cpp:400-413
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]]. 'class' is a common ObjC method selector, so allow it as
+  // well.
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||

benhamilton wrote:
> Maybe we should check the token before AttrTok to see if it's `tok::at`, 
> rather than checking for an identifier followed by `tok::kw_class`?
> 
> I don't think there's any valid C++11 attribute specifier sequence of `@[[]]`.
> 
Ok, sure. I considered that at first but I'm not familiar with C++11 enough to 
say. I thought it wasn't valid, but I wasn't sure.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:400-413
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]]. 'class' is a common ObjC method selector, so allow it as
+  // well.
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||

Maybe we should check the token before AttrTok to see if it's `tok::at`, rather 
than checking for an identifier followed by `tok::kw_class`?

I don't think there's any valid C++11 attribute specifier sequence of `@[[]]`.



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

Thanks for the fix. One question: how does the real Clang parser deal with this 
case? Is it something that's actually ambiguous in the ObjC++ grammar, I wonder?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D64632/new/

https://reviews.llvm.org/D64632



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64632: [clang-format] Don't detect call to ObjC class method as C++11 attribute specifier

[Robbie Gibson via Phabricator via cfe-commits]

rkgibson2 created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64632

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,10 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake a call to the Objective-C method named
+  // "class" inside an array literal as attributes.
+  verifyFormat("@[ [NSArray class] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -400,9 +400,11 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]]. 'class' is a common ObjC method selector, so allow it as
+  // well.
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
+  AttrTok->startsSequence(tok::identifier, tok::kw_class) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -7027,6 +7027,10 @@
   // On the other hand, we still need to correctly find array subscripts.
   verifyFormat("int a = std::vector{1, 2, 3}[0];");
 
+  // Make sure that we do not mistake a call to the Objective-C method named
+  // "class" inside an array literal as attributes.
+  verifyFormat("@[ [NSArray class] ];");
+
   // Make sure we do not parse attributes as lambda introducers.
   FormatStyle MultiLineFunctions = getLLVMStyle();
   MultiLineFunctions.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_None;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -400,9 +400,11 @@
 while (AttrTok && !AttrTok->startsSequence(tok::r_square, tok::r_square)) {
   // ObjC message send. We assume nobody will use : in a C++11 attribute
   // specifier parameter, although this is technically valid:
-  // [[foo(:)]]
+  // [[foo(:)]]. 'class' is a common ObjC method selector, so allow it as
+  // well.
   if (AttrTok->is(tok::colon) ||
   AttrTok->startsSequence(tok::identifier, tok::identifier) ||
+  AttrTok->startsSequence(tok::identifier, tok::kw_class) ||
   AttrTok->startsSequence(tok::r_paren, tok::identifier))
 return false;
   if (AttrTok->is(tok::ellipsis))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits