[PATCH] D69164: [clang-format] fix regression recognizing casts in Obj-C calls

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

rC373922: [clang-format] [PR27004] omits leading space for noexcept when 
formatting…  was to fix 
https://bugs.llvm.org/show_bug.cgi?id=27004 which wasn't just related to 
`delete` it occurred in other cases. (operator++) and there could be other 
cases i guess.

I think the check for delete above the code that was added was checking for 
`delete` being on the left i.e.  `delete()` and `delete(x)` is not a cast  
(sorry I wasn't the author for that part, so might not be correct)

However, I appreciate this patch (or any patch for that matter) that 
strengthens our tests in this way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69164



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

[PATCH] D69164: [clang-format] fix regression recognizing casts in Obj-C calls

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir marked an inline comment as done.
krasimir added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:1612
+  Keywords.kw_final) ||
+isCpp11AttributeSpecifier(*Tok.Next))
   return false;

rdwampler wrote:
> I think the issue r373922 was fixing is only related to the `delete`. Can 
> this check be move further up where we explicitly check if the token is the 
> delete keyword? 
> 
Sorry, I didn't notice this comment until now. I can investigate whether all 
the other cases for non-delete methods are already covered, it would be 
interesting. + @MyDeveloperDay, who might have better insights into this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69164



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

[PATCH] D69164: [clang-format] fix regression recognizing casts in Obj-C calls

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGeff2a2ab2b51: [clang-format] fix regression recognizing 
casts in Obj-C calls (authored by krasimir).

Changed prior to commit:
  https://reviews.llvm.org/D69164?vs=225604&id=225635#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69164

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
@@ -7541,6 +7541,8 @@
   verifyFormat("my_int a = (ns::my_int)-2;");
   verifyFormat("case (my_int)ONE:");
   verifyFormat("auto x = (X)this;");
+  // Casts in Obj-C style calls used to not be recognized as such.
+  verifyFormat("int a = [(type*)[((type*)val) arg] arg];", getGoogleStyle());
 
   // FIXME: single value wrapped with paren will be treated as cast.
   verifyFormat("void f(int i = (kValue)*kMask) {}");
@@ -7581,6 +7583,29 @@
   verifyFormat("int a = alignof(int *) + b;", getGoogleStyle());
   verifyFormat("bool b = f(g) && c;");
   verifyFormat("typedef void (*f)(int i) func;");
+  verifyFormat("void operator++(int) noexcept;");
+  verifyFormat("void operator++(int &) noexcept;");
+  verifyFormat("void operator delete(void *, std::size_t, const std::nothrow_t 
"
+   "&) noexcept;");
+  verifyFormat(
+  "void operator delete(std::size_t, const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(foo &) noexcept;");
+  verifyFormat("void operator delete(foo) noexcept;");
+  verifyFormat("void operator delete(int) noexcept;");
+  verifyFormat("void operator delete(int &) noexcept;");
+  verifyFormat("void operator delete(int &) volatile noexcept;");
+  verifyFormat("void operator delete(int &) const");
+  verifyFormat("void operator delete(int &) = default");
+  verifyFormat("void operator delete(int &) = delete");
+  verifyFormat("void operator delete(int &) [[noreturn]]");
+  verifyFormat("void operator delete(int &) throw();");
+  verifyFormat("void operator delete(int &) throw(int);");
+  verifyFormat("auto operator delete(int &) -> int;");
+  verifyFormat("auto operator delete(int &) override");
+  verifyFormat("auto operator delete(int &) final");
 
   verifyFormat(" *foo = (a 
*)\n"
"bbb;");
@@ -14696,33 +14721,6 @@
   */
 }
 
-TEST_F(FormatTest, NotCastRPaen) {
-
-  verifyFormat("void operator++(int) noexcept;");
-  verifyFormat("void operator++(int &) noexcept;");
-  verifyFormat("void operator delete(void *, std::size_t, const std::nothrow_t 
"
-   "&) noexcept;");
-  verifyFormat(
-  "void operator delete(std::size_t, const std::nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(const std::nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(std::nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(foo &) noexcept;");
-  verifyFormat("void operator delete(foo) noexcept;");
-  verifyFormat("void operator delete(int) noexcept;");
-  verifyFormat("void operator delete(int &) noexcept;");
-  verifyFormat("void operator delete(int &) volatile noexcept;");
-  verifyFormat("void operator delete(int &) const");
-  verifyFormat("void operator delete(int &) = default");
-  verifyFormat("void operator delete(int &) = delete");
-  verifyFormat("void operator delete(int &) [[noreturn]]");
-  verifyFormat("void operator delete(int &) throw();");
-  verifyFormat("void operator delete(int &) throw(int);");
-  verifyFormat("auto operator delete(int &) -> int;");
-  verifyFormat("auto operator delete(int &) override");
-  verifyFormat("auto operator delete(int &) final");
-}
-
 TEST_F(FormatTest, STLWhileNotDefineChed) {
   verifyFormat("#if defined(while)\n"
"#define while EMIT WARNING C4005\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1607,8 +1607,9 @@
 // Functions which end with decorations like volatile, noexcept are 
unlikely
 // to be casts.
 if (Tok.Next->isOneOf(tok::kw_noexcept, tok::kw_volatile, tok::kw_const,
-  tok::kw_throw, tok::l_square, tok::arrow,
-  Keywords.kw_override, Keywords.kw_final))
+  tok::kw_throw, tok::arrow, Keywords.kw_override,
+  Keywords.kw_fin

[PATCH] D69164: [clang-format] fix regression recognizing casts in Obj-C calls

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:1612
+  Keywords.kw_final) ||
+isCpp11AttributeSpecifier(*Tok.Next))
   return false;

I think the issue r373922 was fixing is only related to the `delete`. Can this 
check be move further up where we explicitly check if the token is the delete 
keyword? 



Repository:
  rC Clang

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

https://reviews.llvm.org/D69164



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

[PATCH] D69164: [clang-format] fix regression recognizing casts in Obj-C calls

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

LGTM, thanks for the patch


Repository:
  rC Clang

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

https://reviews.llvm.org/D69164



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

[PATCH] D69164: [clang-format] fix regression recognizing casts in Obj-C calls

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
krasimir edited the summary of this revision.
krasimir added a reviewer: MyDeveloperDay.

r373922 added checks for a few tokens that, following an `)` make it
unlikely that the `)` is the closing paren of a cast expression. The
specific check for `tok::l_square` there introduced a regression for
casts of Obj-C calls, like:

  (cast)[func arg]

>From the tests added in r373922, I believe the `tok::l_square` case is added to
capture the case where a non-cast `)` is directly followed by an
attribute specifier, like:

  int f(int x) [[noreturn]];

I've specialized the code to look for such attribute specifier instead
of `tok::l_square` in general. Also, I added a regression test and moved
the test cases added in r373922 to an already existing place documenting
other instances of historically misidentified casts.


Repository:
  rC Clang

https://reviews.llvm.org/D69164

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


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -7541,6 +7541,8 @@
   verifyFormat("my_int a = (ns::my_int)-2;");
   verifyFormat("case (my_int)ONE:");
   verifyFormat("auto x = (X)this;");
+  // Casts in Obj-C style calls used to not be recognized as such.
+  verifyFormat("int a = [(type*)[((type*)val) arg] arg];", getGoogleStyle());
 
   // FIXME: single value wrapped with paren will be treated as cast.
   verifyFormat("void f(int i = (kValue)*kMask) {}");
@@ -7581,6 +7583,29 @@
   verifyFormat("int a = alignof(int *) + b;", getGoogleStyle());
   verifyFormat("bool b = f(g) && c;");
   verifyFormat("typedef void (*f)(int i) func;");
+  verifyFormat("void operator++(int) noexcept;");
+  verifyFormat("void operator++(int &) noexcept;");
+  verifyFormat("void operator delete(void *, std::size_t, const std::nothrow_t 
"
+   "&) noexcept;");
+  verifyFormat(
+  "void operator delete(std::size_t, const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(const std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(std::nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(nothrow_t &) noexcept;");
+  verifyFormat("void operator delete(foo &) noexcept;");
+  verifyFormat("void operator delete(foo) noexcept;");
+  verifyFormat("void operator delete(int) noexcept;");
+  verifyFormat("void operator delete(int &) noexcept;");
+  verifyFormat("void operator delete(int &) volatile noexcept;");
+  verifyFormat("void operator delete(int &) const");
+  verifyFormat("void operator delete(int &) = default");
+  verifyFormat("void operator delete(int &) = delete");
+  verifyFormat("void operator delete(int &) [[noreturn]]");
+  verifyFormat("void operator delete(int &) throw();");
+  verifyFormat("void operator delete(int &) throw(int);");
+  verifyFormat("auto operator delete(int &) -> int;");
+  verifyFormat("auto operator delete(int &) override");
+  verifyFormat("auto operator delete(int &) final");
 
   verifyFormat(" *foo = (a 
*)\n"
"bbb;");
@@ -14696,33 +14721,6 @@
   */
 }
 
-TEST_F(FormatTest, NotCastRPaen) {
-
-  verifyFormat("void operator++(int) noexcept;");
-  verifyFormat("void operator++(int &) noexcept;");
-  verifyFormat("void operator delete(void *, std::size_t, const std::nothrow_t 
"
-   "&) noexcept;");
-  verifyFormat(
-  "void operator delete(std::size_t, const std::nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(const std::nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(std::nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(nothrow_t &) noexcept;");
-  verifyFormat("void operator delete(foo &) noexcept;");
-  verifyFormat("void operator delete(foo) noexcept;");
-  verifyFormat("void operator delete(int) noexcept;");
-  verifyFormat("void operator delete(int &) noexcept;");
-  verifyFormat("void operator delete(int &) volatile noexcept;");
-  verifyFormat("void operator delete(int &) const");
-  verifyFormat("void operator delete(int &) = default");
-  verifyFormat("void operator delete(int &) = delete");
-  verifyFormat("void operator delete(int &) [[noreturn]]");
-  verifyFormat("void operator delete(int &) throw();");
-  verifyFormat("void operator delete(int &) throw(int);");
-  verifyFormat("auto operator delete(int &) -> int;");
-  verifyFormat("auto operator delete(int &) override");
-  verifyFormat("auto operator delete(int &) final");
-}
-
 TEST_F(FormatTest, STLWhileNotDefineChed) {
   verifyFormat("#if defined(while)\n"
"#define while EMIT WARNING C4005\n"
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Form