[PATCH] D151578: [Format/ObjC] Support NS_ASSUME_NONNULL_BEGIN and FOUNDATION_EXPORT in ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG85670ac86813: [Format/ObjC] Support NS_ASSUME_NONNULL_BEGIN 
and FOUNDATION_EXPORT in ObjC… (authored by benhamilton).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151578

Files:
  clang/lib/Format/Format.cpp
  clang/test/Format/dump-config-objc-macros.h
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -94,6 +94,26 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("{}", "a.h", "none", R"objc(
+NS_ASSUME_NONNULL_BEGIN
+extern int i;
+NS_ASSUME_NONNULL_END
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXTERN void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXPORT void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/test/Format/dump-config-objc-macros.h
===
--- /dev/null
+++ clang/test/Format/dump-config-objc-macros.h
@@ -0,0 +1,8 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+NS_ASSUME_NONNULL_BEGIN
+
+FOUNDATION_EXTERN int kConstant;
+
+NS_ASSUME_NONNULL_END
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2687,6 +2687,8 @@
 "CGSizeMake",
 "CGVector",
 "CGVectorMake",
+"FOUNDATION_EXPORT", // This is an alias for FOUNDATION_EXTERN.
+"FOUNDATION_EXTERN",
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
@@ -2743,6 +2745,7 @@
 "NSURLQueryItem",
 "NSUUID",
 "NSValue",
+"NS_ASSUME_NONNULL_BEGIN",
 "UIImage",
 "UIView",
 };


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -94,6 +94,26 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("{}", "a.h", "none", R"objc(
+NS_ASSUME_NONNULL_BEGIN
+extern int i;
+NS_ASSUME_NONNULL_END
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXTERN void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXPORT void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/test/Format/dump-config-objc-macros.h
===
--- /dev/null
+++ clang/test/Format/dump-config-objc-macros.h
@@ -0,0 +1,8 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+NS_ASSUME_NONNULL_BEGIN
+
+FOUNDATION_EXTERN int kConstant;
+
+NS_ASSUME_NONNULL_END
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2687,6 +2687,8 @@
 "CGSizeMake",
 "CGVector",
 "CGVectorMake",
+"FOUNDATION_EXPORT", // This is an alias for FOUNDATION_EXTERN.
+"FOUNDATION_EXTERN",
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
@@ -2743,6 +2745,7 @@
 "NSURLQueryItem",
 "NSUUID",
 "NSValue",
+"NS_ASSUME_NONNULL_BEGIN",
 "UIImage",
 "UIView",
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151578: [Format/ObjC] Support NS_ASSUME_NONNULL_BEGIN and FOUNDATION_EXPORT in ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 526154.
benhamilton added a comment.

Add `FOUNDATION_EXTERN` alias for `FOUNDATION_EXPORT` (Apple `#define`s the 
former to the latter).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151578

Files:
  clang/lib/Format/Format.cpp
  clang/test/Format/dump-config-objc-macros.h
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -94,6 +94,26 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("{}", "a.h", "none", R"objc(
+NS_ASSUME_NONNULL_BEGIN
+extern int i;
+NS_ASSUME_NONNULL_END
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXTERN void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXPORT void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/test/Format/dump-config-objc-macros.h
===
--- /dev/null
+++ clang/test/Format/dump-config-objc-macros.h
@@ -0,0 +1,8 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+NS_ASSUME_NONNULL_BEGIN
+
+FOUNDATION_EXTERN int kConstant;
+
+NS_ASSUME_NONNULL_END
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2687,6 +2687,8 @@
 "CGSizeMake",
 "CGVector",
 "CGVectorMake",
+"FOUNDATION_EXPORT", // This is an alias for FOUNDATION_EXTERN.
+"FOUNDATION_EXTERN",
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
@@ -2743,6 +2745,7 @@
 "NSURLQueryItem",
 "NSUUID",
 "NSValue",
+"NS_ASSUME_NONNULL_BEGIN",
 "UIImage",
 "UIView",
 };


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -94,6 +94,26 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("{}", "a.h", "none", R"objc(
+NS_ASSUME_NONNULL_BEGIN
+extern int i;
+NS_ASSUME_NONNULL_END
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXTERN void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXPORT void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/test/Format/dump-config-objc-macros.h
===
--- /dev/null
+++ clang/test/Format/dump-config-objc-macros.h
@@ -0,0 +1,8 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+NS_ASSUME_NONNULL_BEGIN
+
+FOUNDATION_EXTERN int kConstant;
+
+NS_ASSUME_NONNULL_END
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2687,6 +2687,8 @@
 "CGSizeMake",
 "CGVector",
 "CGVectorMake",
+"FOUNDATION_EXPORT", // This is an alias for FOUNDATION_EXTERN.
+"FOUNDATION_EXTERN",
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
@@ -2743,6 +2745,7 @@
 "NSURLQueryItem",
 "NSUUID",
 "NSValue",
+"NS_ASSUME_NONNULL_BEGIN",
 "UIImage",
 "UIView",
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151578: [Format/ObjC] Support NS_ASSUME_NONNULL_BEGIN and FOUNDATION_EXPORT in ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 526151.
benhamilton added a comment.

Fix auto-complete-o.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151578

Files:
  clang/lib/Format/Format.cpp
  clang/test/Format/dump-config-objc-macros.h
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -94,6 +94,20 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("{}", "a.h", "none", R"objc(
+NS_ASSUME_NONNULL_BEGIN
+extern int i;
+NS_ASSUME_NONNULL_END
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXTERN void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/test/Format/dump-config-objc-macros.h
===
--- /dev/null
+++ clang/test/Format/dump-config-objc-macros.h
@@ -0,0 +1,8 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+NS_ASSUME_NONNULL_BEGIN
+
+FOUNDATION_EXTERN int kConstant;
+
+NS_ASSUME_NONNULL_END
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2687,6 +2687,7 @@
 "CGSizeMake",
 "CGVector",
 "CGVectorMake",
+"FOUNDATION_EXTERN",
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
@@ -2743,6 +2744,7 @@
 "NSURLQueryItem",
 "NSUUID",
 "NSValue",
+"NS_ASSUME_NONNULL_BEGIN",
 "UIImage",
 "UIView",
 };


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -94,6 +94,20 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("{}", "a.h", "none", R"objc(
+NS_ASSUME_NONNULL_BEGIN
+extern int i;
+NS_ASSUME_NONNULL_END
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXTERN void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/test/Format/dump-config-objc-macros.h
===
--- /dev/null
+++ clang/test/Format/dump-config-objc-macros.h
@@ -0,0 +1,8 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+NS_ASSUME_NONNULL_BEGIN
+
+FOUNDATION_EXTERN int kConstant;
+
+NS_ASSUME_NONNULL_END
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2687,6 +2687,7 @@
 "CGSizeMake",
 "CGVector",
 "CGVectorMake",
+"FOUNDATION_EXTERN",
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
@@ -2743,6 +2744,7 @@
 "NSURLQueryItem",
 "NSUUID",
 "NSValue",
+"NS_ASSUME_NONNULL_BEGIN",
 "UIImage",
 "UIView",
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D151578: [Format/ObjC] Support NS_ASSUME_NONNULL_BEGIN and FOUNDATION_EXPORT in ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added a reviewer: MyDeveloperDay.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan.
benhamilton requested review of this revision.

This adds to the ObjC language guesser a few more common macros used
in ObjC headers. These can help distinguish ObjC headers which
otherwise lack ObjC types from C++ headers.

Contributed by danblakemore.

Tested:

  New tests included. Ran unit tests with:
  ```
  % cmake -S llvm -B build -G Ninja && \
ninja -C build FormatTests && \
./build/tools/clang/unittests/Format/FormatTests 
--gtest_filter="*FormatTestObjC*"
  
  (snip)
  [--] 24 tests from FormatTestObjC (265 ms total)
  
  [--] Global test environment tear-down
  [==] 26 tests from 2 test suites ran. (270 ms total)
  [  PASSED  ] 26 tests.
  ```


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D151578

Files:
  clang/lib/Format/Format.cpp
  clang/test/Format/dump-config-objc-macros.h
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -94,6 +94,20 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("{}", "a.h", "none", R"objc(
+NS_ASSUME_NONNULL_BEGIN
+extern int i;
+NS_ASSUME_NONNULL_END
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXTERN void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/test/Format/dump-config-objc-macros.h
===
--- /dev/null
+++ clang/test/Format/dump-config-objc-macros.h
@@ -0,0 +1,8 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+NS_ASSUME_NONNULL_BEGIN
+
+FOUNDATION_EXPORT int kConstant;
+
+NS_ASSUME_NONNULL_END
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2687,6 +2687,7 @@
 "CGSizeMake",
 "CGVector",
 "CGVectorMake",
+"FOUNDATION_EXTERN",
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
@@ -2743,6 +2744,7 @@
 "NSURLQueryItem",
 "NSUUID",
 "NSValue",
+"NS_ASSUME_NONNULL_BEGIN",
 "UIImage",
 "UIView",
 };


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -94,6 +94,20 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style = getStyle("{}", "a.h", "none", R"objc(
+NS_ASSUME_NONNULL_BEGIN
+extern int i;
+NS_ASSUME_NONNULL_END
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none", R"objc(
+FOUNDATION_EXTERN void DoStuff(void);
+)objc");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/test/Format/dump-config-objc-macros.h
===
--- /dev/null
+++ clang/test/Format/dump-config-objc-macros.h
@@ -0,0 +1,8 @@
+// RUN: clang-format -dump-config %s | FileCheck %s
+
+// CHECK: Language: ObjC
+NS_ASSUME_NONNULL_BEGIN
+
+FOUNDATION_EXPORT int kConstant;
+
+NS_ASSUME_NONNULL_END
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2687,6 +2687,7 @@
 "CGSizeMake",
 "CGVector",
 "CGVectorMake",
+"FOUNDATION_EXTERN",
 "NSAffineTransform",
 "NSArray",
 "NSAttributedString",
@@ -2743,6 +2744,7 @@
 "NSURLQueryItem",
 "NSUUID",
 "NSValue",
+"NS_ASSUME_NONNULL_BEGIN",
 "UIImage",
 "UIView",
 };
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147577: [Format/ObjC] Support NS_ERROR_ENUM in ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a subscriber: goncharov.
benhamilton added a comment.

In D147577#4252225 , @MyDeveloperDay 
wrote:

> The build machines aren’t using a latest enough version of clang-format for 
> our local .clang-format file

Thanks, that's what I figured. I assigned 
https://github.com/llvm/llvm-project/issues/62007 to @goncharov for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147577

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

[PATCH] D147577: [Format/ObjC] Support NS_ERROR_ENUM in ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG00ea6798959d: [Format/ObjC] Support NS_ERROR_ENUM in ObjC 
language guesser (authored by benhamilton).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147577

Files:
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -89,6 +89,11 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style =
+  getStyle("{}", "a.h", "none", "typedef NS_ERROR_ENUM(int, Foo) {};\n");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -949,6 +949,7 @@
 kw_CF_OPTIONS = &IdentTable.get("CF_OPTIONS");
 kw_NS_CLOSED_ENUM = &IdentTable.get("NS_CLOSED_ENUM");
 kw_NS_ENUM = &IdentTable.get("NS_ENUM");
+kw_NS_ERROR_ENUM = &IdentTable.get("NS_ERROR_ENUM");
 kw_NS_OPTIONS = &IdentTable.get("NS_OPTIONS");
 
 kw_as = &IdentTable.get("as");
@@ -1334,6 +1335,7 @@
   IdentifierInfo *kw_CF_OPTIONS;
   IdentifierInfo *kw_NS_CLOSED_ENUM;
   IdentifierInfo *kw_NS_ENUM;
+  IdentifierInfo *kw_NS_ERROR_ENUM;
   IdentifierInfo *kw_NS_OPTIONS;
   IdentifierInfo *kw___except;
   IdentifierInfo *kw___has_include;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2699,6 +2699,8 @@
 "NSDecimalNumber",
 "NSDictionary",
 "NSEdgeInsets",
+"NSError",
+"NSErrorDomain",
 "NSHashTable",
 "NSIndexPath",
 "NSIndexSet",
@@ -2760,6 +2762,7 @@
 FormatTok->TokenText)) ||
 FormatTok->is(TT_ObjCStringLiteral) ||
 FormatTok->isOneOf(Keywords.kw_NS_CLOSED_ENUM, Keywords.kw_NS_ENUM,
+   Keywords.kw_NS_ERROR_ENUM,
Keywords.kw_NS_OPTIONS, TT_ObjCBlockLBrace,
TT_ObjCBlockLParen, TT_ObjCDecl, TT_ObjCForIn,
TT_ObjCMethodExpr, TT_ObjCMethodSpecifier,


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -89,6 +89,11 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style =
+  getStyle("{}", "a.h", "none", "typedef NS_ERROR_ENUM(int, Foo) {};\n");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -949,6 +949,7 @@
 kw_CF_OPTIONS = &IdentTable.get("CF_OPTIONS");
 kw_NS_CLOSED_ENUM = &IdentTable.get("NS_CLOSED_ENUM");
 kw_NS_ENUM = &IdentTable.get("NS_ENUM");
+kw_NS_ERROR_ENUM = &IdentTable.get("NS_ERROR_ENUM");
 kw_NS_OPTIONS = &IdentTable.get("NS_OPTIONS");
 
 kw_as = &IdentTable.get("as");
@@ -1334,6 +1335,7 @@
   IdentifierInfo *kw_CF_OPTIONS;
   IdentifierInfo *kw_NS_CLOSED_ENUM;
   IdentifierInfo *kw_NS_ENUM;
+  IdentifierInfo *kw_NS_ERROR_ENUM;
   IdentifierInfo *kw_NS_OPTIONS;
   IdentifierInfo *kw___except;
   IdentifierInfo *kw___has_include;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2699,6 +2699,8 @@
 "NSDecimalNumber",
 "NSDictionary",
 "NSEdgeInsets",
+"NSError",
+"NSErrorDomain",
 "NSHashTable",
 "NSIndexPath",
 "NSIndexSet",
@@ -2760,6 +2762,7 @@
 FormatTok->TokenText)) ||
 FormatTok->is(TT_ObjCStringLiteral) ||
 FormatTok->isOneOf(Keywords.kw_NS_CLOSED_ENUM, Keywords.kw_NS_ENUM,
+   Keywords.kw_NS_ERROR_ENUM,
Keywords.kw_NS_OPTIONS, TT_ObjCBlockLBrace,
TT_ObjCBlockLParen, TT_ObjCDecl, TT_ObjCForIn,
 

[PATCH] D147577: [Format/ObjC] Support NS_ERROR_ENUM in ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

Filed https://github.com/llvm/llvm-project/issues/62007 on the buildkite 
failure (despite it being in `git-clang-format`, I'm about 98% sure it's 
unrelated).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147577

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

[PATCH] D147577: [Format/ObjC] Support NS_ERROR_ENUM in ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 511539.
benhamilton added a comment.

Rebasing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147577

Files:
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -89,6 +89,11 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style =
+  getStyle("{}", "a.h", "none", "typedef NS_ERROR_ENUM(int, Foo) {};\n");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -949,6 +949,7 @@
 kw_CF_OPTIONS = &IdentTable.get("CF_OPTIONS");
 kw_NS_CLOSED_ENUM = &IdentTable.get("NS_CLOSED_ENUM");
 kw_NS_ENUM = &IdentTable.get("NS_ENUM");
+kw_NS_ERROR_ENUM = &IdentTable.get("NS_ERROR_ENUM");
 kw_NS_OPTIONS = &IdentTable.get("NS_OPTIONS");
 
 kw_as = &IdentTable.get("as");
@@ -1334,6 +1335,7 @@
   IdentifierInfo *kw_CF_OPTIONS;
   IdentifierInfo *kw_NS_CLOSED_ENUM;
   IdentifierInfo *kw_NS_ENUM;
+  IdentifierInfo *kw_NS_ERROR_ENUM;
   IdentifierInfo *kw_NS_OPTIONS;
   IdentifierInfo *kw___except;
   IdentifierInfo *kw___has_include;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2699,6 +2699,8 @@
 "NSDecimalNumber",
 "NSDictionary",
 "NSEdgeInsets",
+"NSError",
+"NSErrorDomain",
 "NSHashTable",
 "NSIndexPath",
 "NSIndexSet",
@@ -2760,6 +2762,7 @@
 FormatTok->TokenText)) ||
 FormatTok->is(TT_ObjCStringLiteral) ||
 FormatTok->isOneOf(Keywords.kw_NS_CLOSED_ENUM, Keywords.kw_NS_ENUM,
+   Keywords.kw_NS_ERROR_ENUM,
Keywords.kw_NS_OPTIONS, TT_ObjCBlockLBrace,
TT_ObjCBlockLParen, TT_ObjCDecl, TT_ObjCForIn,
TT_ObjCMethodExpr, TT_ObjCMethodSpecifier,


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -89,6 +89,11 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style =
+  getStyle("{}", "a.h", "none", "typedef NS_ERROR_ENUM(int, Foo) {};\n");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -949,6 +949,7 @@
 kw_CF_OPTIONS = &IdentTable.get("CF_OPTIONS");
 kw_NS_CLOSED_ENUM = &IdentTable.get("NS_CLOSED_ENUM");
 kw_NS_ENUM = &IdentTable.get("NS_ENUM");
+kw_NS_ERROR_ENUM = &IdentTable.get("NS_ERROR_ENUM");
 kw_NS_OPTIONS = &IdentTable.get("NS_OPTIONS");
 
 kw_as = &IdentTable.get("as");
@@ -1334,6 +1335,7 @@
   IdentifierInfo *kw_CF_OPTIONS;
   IdentifierInfo *kw_NS_CLOSED_ENUM;
   IdentifierInfo *kw_NS_ENUM;
+  IdentifierInfo *kw_NS_ERROR_ENUM;
   IdentifierInfo *kw_NS_OPTIONS;
   IdentifierInfo *kw___except;
   IdentifierInfo *kw___has_include;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2699,6 +2699,8 @@
 "NSDecimalNumber",
 "NSDictionary",
 "NSEdgeInsets",
+"NSError",
+"NSErrorDomain",
 "NSHashTable",
 "NSIndexPath",
 "NSIndexSet",
@@ -2760,6 +2762,7 @@
 FormatTok->TokenText)) ||
 FormatTok->is(TT_ObjCStringLiteral) ||
 FormatTok->isOneOf(Keywords.kw_NS_CLOSED_ENUM, Keywords.kw_NS_ENUM,
+   Keywords.kw_NS_ERROR_ENUM,
Keywords.kw_NS_OPTIONS, TT_ObjCBlockLBrace,
TT_ObjCBlockLParen, TT_ObjCDecl, TT_ObjCForIn,
TT_ObjCMethodExpr, TT_ObjCMethodSpecifier,
___
cfe-commits mailing list
cfe-commits@lists.

[PATCH] D147577: [Format/ObjC] Support NS_ERROR_ENUM in ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: sammccall, MyDeveloperDay.
Herald added projects: All, clang, clang-format.
Herald added a subscriber: cfe-commits.
Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan.
benhamilton requested review of this revision.

Apple added a new NS_ERROR_ENUM macro to help define enums for
NSError codes.

This updates libformat's Objective-C language-guessing heuristic
to detect the new macro as well as related NSError types.

Tested: New tests added.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147577

Files:
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -89,6 +89,11 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style =
+  getStyle("{}", "a.h", "none", "typedef NS_ERROR_ENUM(int, Foo) {};\n");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -949,6 +949,7 @@
 kw_CF_OPTIONS = &IdentTable.get("CF_OPTIONS");
 kw_NS_CLOSED_ENUM = &IdentTable.get("NS_CLOSED_ENUM");
 kw_NS_ENUM = &IdentTable.get("NS_ENUM");
+kw_NS_ERROR_ENUM = &IdentTable.get("NS_ERROR_ENUM");
 kw_NS_OPTIONS = &IdentTable.get("NS_OPTIONS");
 
 kw_as = &IdentTable.get("as");
@@ -1334,6 +1335,7 @@
   IdentifierInfo *kw_CF_OPTIONS;
   IdentifierInfo *kw_NS_CLOSED_ENUM;
   IdentifierInfo *kw_NS_ENUM;
+  IdentifierInfo *kw_NS_ERROR_ENUM;
   IdentifierInfo *kw_NS_OPTIONS;
   IdentifierInfo *kw___except;
   IdentifierInfo *kw___has_include;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2699,6 +2699,8 @@
 "NSDecimalNumber",
 "NSDictionary",
 "NSEdgeInsets",
+"NSError",
+"NSErrorDomain",
 "NSHashTable",
 "NSIndexPath",
 "NSIndexSet",
@@ -2760,6 +2762,7 @@
 FormatTok->TokenText)) ||
 FormatTok->is(TT_ObjCStringLiteral) ||
 FormatTok->isOneOf(Keywords.kw_NS_CLOSED_ENUM, Keywords.kw_NS_ENUM,
+   Keywords.kw_NS_ERROR_ENUM,
Keywords.kw_NS_OPTIONS, TT_ObjCBlockLBrace,
TT_ObjCBlockLParen, TT_ObjCDecl, TT_ObjCForIn,
TT_ObjCMethodExpr, TT_ObjCMethodSpecifier,


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -89,6 +89,11 @@
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
+  Style =
+  getStyle("{}", "a.h", "none", "typedef NS_ERROR_ENUM(int, Foo) {};\n");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
   Style = getStyle("{}", "a.h", "none", "enum Foo {};");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
Index: clang/lib/Format/FormatToken.h
===
--- clang/lib/Format/FormatToken.h
+++ clang/lib/Format/FormatToken.h
@@ -949,6 +949,7 @@
 kw_CF_OPTIONS = &IdentTable.get("CF_OPTIONS");
 kw_NS_CLOSED_ENUM = &IdentTable.get("NS_CLOSED_ENUM");
 kw_NS_ENUM = &IdentTable.get("NS_ENUM");
+kw_NS_ERROR_ENUM = &IdentTable.get("NS_ERROR_ENUM");
 kw_NS_OPTIONS = &IdentTable.get("NS_OPTIONS");
 
 kw_as = &IdentTable.get("as");
@@ -1334,6 +1335,7 @@
   IdentifierInfo *kw_CF_OPTIONS;
   IdentifierInfo *kw_NS_CLOSED_ENUM;
   IdentifierInfo *kw_NS_ENUM;
+  IdentifierInfo *kw_NS_ERROR_ENUM;
   IdentifierInfo *kw_NS_OPTIONS;
   IdentifierInfo *kw___except;
   IdentifierInfo *kw___has_include;
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -2699,6 +2699,8 @@
 "NSDecimalNumber",
 "NSDictionary",
 "NSEdgeInsets",
+"NSError",
+"NSErrorDomain",
 "NSHashTable",
 "NSIndexPath",
 "NSIndexSet",
@@ -2760,6 +2762,7 @@
 FormatTok->TokenText)) ||
 FormatTok->is(TT_ObjCStringLiteral) ||
 FormatTok->isOneOf(Keywords.kw_NS_CLOSED_ENUM, Keywords.kw_NS_ENUM,
+   

[PATCH] D118921: [Format] Don't derive pointers right based on space before method ref-qualifiers

[Ben Hamilton via Phabricator via cfe-commits]

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

Got it, thanks.




Comment at: clang/lib/Format/Format.cpp:1949
   continue;
+// Don't treat space in `void foo() &&` as evidence.
+if (const auto *Prev = Tok->getPreviousNonComment()) {

sammccall wrote:
> benhamilton wrote:
> > Do we also need to worry about `T && foo();` style declarations?
> > 
> A space before && in that case *should* trigger right-alignment.
> 
> Are you concerned the exclusion here might fire?
No, just wanted to make sure it should trigger right-alignment (which is what 
this implements, of course.)




Comment at: clang/lib/Format/Format.cpp:1951
+if (const auto *Prev = Tok->getPreviousNonComment()) {
+  if (Prev->is(tok::r_paren) && Prev->MatchingParen)
+if (const auto *Func = 
Prev->MatchingParen->getPreviousNonComment())

sammccall wrote:
> benhamilton wrote:
> > If this is parsing a general function declaration, I think it's not safe to 
> > assume that the token immediately before the ref-spec is the close-paren.
> > 
> > It looks like `Prev` can be a few other things, including at least:
> > 
> > * `tok::kw_const`
> > * `tok::kw_volatile`
> > * `tok::kw_throw`
> > * `tok::kw_noexcept`
> > 
> > and probably C++ attribute(s) as well:
> > 
> > > noptr-declarator ( parameter-list ) cv(optional) ref(optional) 
> > > except(optional) attr(optional)
> > 
> > * https://en.cppreference.com/w/cpp/language/function
> > * https://en.cppreference.com/w/cpp/language/declarations
> > 
> > I *think* they can also be in any order, so the ref-spec could come at the 
> > very end after cv-qualifier, an expection specification, and C++ attributes.
> > 
> const/volatile: a space between const and & does indicate right alignment. 
> (And clang-format emits a space under PAS_Right)
> 
> throw etc: these are not possible, there's no "in any order". Here's the 
> grammar: http://eel.is/c++draft/dcl.decl.general#nt:parameters-and-qualifiers
Ah, good to know the grammar is more strict about ordering.



Comment at: clang/unittests/Format/FormatTest.cpp:9689-9692
+  // There's always a space between the function and its trailing qualifiers.
+  // This isn't evidence for PAS_Right (or for PAS_Left).
+  std::string Prefix = "void a() &;\n"
+   "void b() &;\n";

Worth a test with `&&`?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118921

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

[PATCH] D118921: [Format] Don't derive pointers right based on space before method ref-qualifiers

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton requested changes to this revision.
benhamilton added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/Format/Format.cpp:1949
   continue;
+// Don't treat space in `void foo() &&` as evidence.
+if (const auto *Prev = Tok->getPreviousNonComment()) {

Do we also need to worry about `T && foo();` style declarations?




Comment at: clang/lib/Format/Format.cpp:1951
+if (const auto *Prev = Tok->getPreviousNonComment()) {
+  if (Prev->is(tok::r_paren) && Prev->MatchingParen)
+if (const auto *Func = 
Prev->MatchingParen->getPreviousNonComment())

If this is parsing a general function declaration, I think it's not safe to 
assume that the token immediately before the ref-spec is the close-paren.

It looks like `Prev` can be a few other things, including at least:

* `tok::kw_const`
* `tok::kw_volatile`
* `tok::kw_throw`
* `tok::kw_noexcept`

and probably C++ attribute(s) as well:

> noptr-declarator ( parameter-list ) cv(optional) ref(optional) 
> except(optional) attr(optional)

* https://en.cppreference.com/w/cpp/language/function
* https://en.cppreference.com/w/cpp/language/declarations

I *think* they can also be in any order, so the ref-spec could come at the very 
end after cv-qualifier, an expection specification, and C++ attributes.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118921

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

[PATCH] D111975: [clang-format] [PR52015] clang-format should put __attribute__((foo)) on its own line before @interface / @implementation / @protocol

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added a comment.

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111975

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

[PATCH] D99063: [clang-format] Fix ObjC method indent after f7f9f94b

[Ben Hamilton via Phabricator via cfe-commits]

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

Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99063

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

[PATCH] D89496: [Format/ObjC] Correctly handle base class with lightweight generics and protocol

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG24b5266892c3: [Format/ObjC] Correctly handle base class with 
lightweight generics and protocol (authored by benhamilton).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89496

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -328,6 +328,18 @@
"+ (id)init;\n"
"@end");
 
+  verifyFormat("@interface Foo> : Xyzzy   {\n"
+   "  int _i;\n"
+   "}\n"
+   "+ (id)init;\n"
+   "@end");
+
+  verifyFormat("@interface Foo : Bar  \n"
+   "@end");
+
+  verifyFormat("@interface Foo : Bar  \n"
+   "@end");
+
   verifyFormat("@interface Foo (HackStuff) {\n"
"  int _i;\n"
"}\n"
@@ -413,6 +425,10 @@
"f,\n"
"f> {\n"
"}");
+  verifyFormat("@interface g\n"
+   ": g \n"
+   "  \n"
+   "@end");
 }
 
 TEST_F(FormatTestObjC, FormatObjCImplementation) {
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -118,6 +118,7 @@
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCLightweightGenerics();
   void parseObjCMethod();
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2612,32 +2612,15 @@
   // @interface can be followed by a lightweight generic
   // specialization list, then either a base class or a category.
   if (FormatTok->Tok.is(tok::less)) {
-// Unlike protocol lists, generic parameterizations support
-// nested angles:
-//
-// @interface Foo> :
-// NSObject 
-//
-// so we need to count how many open angles we have left.
-unsigned NumOpenAngles = 1;
-do {
-  nextToken();
-  // Early exit in case someone forgot a close angle.
-  if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
-  FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
-break;
-  if (FormatTok->Tok.is(tok::less))
-++NumOpenAngles;
-  else if (FormatTok->Tok.is(tok::greater)) {
-assert(NumOpenAngles > 0 && "'>' makes NumOpenAngles negative");
---NumOpenAngles;
-  }
-} while (!eof() && NumOpenAngles != 0);
-nextToken(); // Skip '>'.
+parseObjCLightweightGenerics();
   }
   if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
 nextToken(); // base class name
+// The base class can also have lightweight generics applied to it.
+if (FormatTok->Tok.is(tok::less)) {
+  parseObjCLightweightGenerics();
+}
   } else if (FormatTok->Tok.is(tok::l_paren))
 // Skip category, if present.
 parseParens();
@@ -2658,6 +2641,32 @@
   parseObjCUntilAtEnd();
 }
 
+void UnwrappedLineParser::parseObjCLightweightGenerics() {
+  assert(FormatTok->Tok.is(tok::less));
+  // Unlike protocol lists, generic parameterizations support
+  // nested angles:
+  //
+  // @interface Foo> :
+  // NSObject 
+  //
+  // so we need to count how many open angles we have left.
+  unsigned NumOpenAngles = 1;
+  do {
+nextToken();
+// Early exit in case someone forgot a close angle.
+if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
+FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
+  break;
+if (FormatTok->Tok.is(tok::less))
+  ++NumOpenAngles;
+else if (FormatTok->Tok.is(tok::greater)) {
+  assert(NumOpenAngles > 0 && "'>' makes NumOpenAngles negative");
+  --NumOpenAngles;
+}
+  } while (!eof() && NumOpenAngles != 0);
+  nextToken(); // Skip '>'.
+}
+
 // Returns true for the declaration/definition form of @protocol,
 // false for the expression form.
 bool UnwrappedLineParser::parseObjCProtocol() {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89496: [Format/ObjC] Correctly handle base class with lightweight generics and protocol

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: sammccall, MyDeveloperDay.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
benhamilton requested review of this revision.

ClangFormat does not correctly handle an Objective-C interface declaration
with both lightweight generics and a protocol conformance.

This simple example:

  @interface Foo : Bar  
  
  @end

means `Foo` extends `Bar` (a lightweight generic class whose type
parameter is `Baz`) and also conforms to the protocol `Blech`.

ClangFormat should not apply any changes to the above example, but
instead it currently formats it quite poorly:

  @interface Foo : Bar 
  
  
  @end
  

The bug is that `UnwrappedLineParser` assumes an open-angle bracket
after a base class name is a protocol list, but it can also be a
lightweight generic specification.

This diff fixes the bug by factoring out the logic to parse
lightweight generics so it can apply both to the declared class
as well as the base class.

Test Plan: New tests added. Ran tests with:

  % ninja FormatTests && ./tools/clang/unittests/Format/FormatTests
  Confirmed tests failed before diff and passed after diff.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89496

Files:
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTestObjC.cpp

Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -328,6 +328,18 @@
"+ (id)init;\n"
"@end");
 
+  verifyFormat("@interface Foo> : Xyzzy   {\n"
+   "  int _i;\n"
+   "}\n"
+   "+ (id)init;\n"
+   "@end");
+
+  verifyFormat("@interface Foo : Bar  \n"
+   "@end");
+
+  verifyFormat("@interface Foo : Bar  \n"
+   "@end");
+
   verifyFormat("@interface Foo (HackStuff) {\n"
"  int _i;\n"
"}\n"
@@ -413,6 +425,10 @@
"f,\n"
"f> {\n"
"}");
+  verifyFormat("@interface g\n"
+   ": g \n"
+   "  \n"
+   "@end");
 }
 
 TEST_F(FormatTestObjC, FormatObjCImplementation) {
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -118,6 +118,7 @@
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCLightweightGenerics();
   void parseObjCMethod();
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -2612,32 +2612,15 @@
   // @interface can be followed by a lightweight generic
   // specialization list, then either a base class or a category.
   if (FormatTok->Tok.is(tok::less)) {
-// Unlike protocol lists, generic parameterizations support
-// nested angles:
-//
-// @interface Foo> :
-// NSObject 
-//
-// so we need to count how many open angles we have left.
-unsigned NumOpenAngles = 1;
-do {
-  nextToken();
-  // Early exit in case someone forgot a close angle.
-  if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
-  FormatTok->Tok.isObjCAtKeyword(tok::objc_end))
-break;
-  if (FormatTok->Tok.is(tok::less))
-++NumOpenAngles;
-  else if (FormatTok->Tok.is(tok::greater)) {
-assert(NumOpenAngles > 0 && "'>' makes NumOpenAngles negative");
---NumOpenAngles;
-  }
-} while (!eof() && NumOpenAngles != 0);
-nextToken(); // Skip '>'.
+parseObjCLightweightGenerics();
   }
   if (FormatTok->Tok.is(tok::colon)) {
 nextToken();
 nextToken(); // base class name
+// The base class can also have lightweight generics applied to it.
+if (FormatTok->Tok.is(tok::less)) {
+  parseObjCLightweightGenerics();
+}
   } else if (FormatTok->Tok.is(tok::l_paren))
 // Skip category, if present.
 parseParens();
@@ -2658,6 +2641,32 @@
   parseObjCUntilAtEnd();
 }
 
+void UnwrappedLineParser::parseObjCLightweightGenerics() {
+  assert(FormatTok->Tok.is(tok::less));
+  // Unlike protocol lists, generic parameterizations support
+  // nested angles:
+  //
+  // @interface Foo> :
+  // NSObject 
+  //
+  // so we need to count how many open angles we have left.
+  unsigned NumOpenAngles = 1;
+  do {
+nextToken();
+// Early exit in case someone forgot a close angle.
+if (FormatTok->isOneOf(tok::semi, tok::l_brace) ||
+  

[PATCH] D89425: [Format/ObjC] Add NS_SWIFT_NAME() and CF_SWIFT_NAME() to WhitespaceSensitiveMacros

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGe7b4feea8e1b: [Format/ObjC] Add NS_SWIFT_NAME() and 
CF_SWIFT_NAME() to… (authored by benhamilton).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89425

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1024,6 +1024,12 @@
   verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;");
   verifyFormat("@property(assign, getter=isEditable) BOOL editable;");
 
+  verifyFormat("extern UIWindow *MainWindow(void) "
+   "NS_SWIFT_NAME(getter:MyHelper.mainWindow());");
+
+  verifyFormat("extern UIWindow *MainWindow(void) "
+   "CF_SWIFT_NAME(getter:MyHelper.mainWindow());");
+
   Style.ColumnLimit = 50;
   verifyFormat("@interface Foo\n"
"- (void)doStuffWithFoo:(id)name\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -964,6 +964,8 @@
   LLVMStyle.WhitespaceSensitiveMacros.push_back("STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("PP_STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("BOOST_PP_STRINGIZE");
+  LLVMStyle.WhitespaceSensitiveMacros.push_back("NS_SWIFT_NAME");
+  LLVMStyle.WhitespaceSensitiveMacros.push_back("CF_SWIFT_NAME");
 
   // Defaults that differ when not C++.
   if (Language == FormatStyle::LK_TableGen) {


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1024,6 +1024,12 @@
   verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;");
   verifyFormat("@property(assign, getter=isEditable) BOOL editable;");
 
+  verifyFormat("extern UIWindow *MainWindow(void) "
+   "NS_SWIFT_NAME(getter:MyHelper.mainWindow());");
+
+  verifyFormat("extern UIWindow *MainWindow(void) "
+   "CF_SWIFT_NAME(getter:MyHelper.mainWindow());");
+
   Style.ColumnLimit = 50;
   verifyFormat("@interface Foo\n"
"- (void)doStuffWithFoo:(id)name\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -964,6 +964,8 @@
   LLVMStyle.WhitespaceSensitiveMacros.push_back("STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("PP_STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("BOOST_PP_STRINGIZE");
+  LLVMStyle.WhitespaceSensitiveMacros.push_back("NS_SWIFT_NAME");
+  LLVMStyle.WhitespaceSensitiveMacros.push_back("CF_SWIFT_NAME");
 
   // Defaults that differ when not C++.
   if (Language == FormatStyle::LK_TableGen) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D89425: [Format/ObjC] Add NS_SWIFT_NAME() and CF_SWIFT_NAME() to WhitespaceSensitiveMacros

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: sammccall, JakeMerdichAMD, curdeius.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
benhamilton requested review of this revision.

The argument passed to the preprocessor macros `NS_SWIFT_NAME(x)` and
`CF_SWIFT_NAME(x)` is stringified before passing to
`__attribute__((swift_name("x")))`.

ClangFormat didn't know about this stringification, so its custom parser
tried to parse the argument(s) passed to the macro as if they were
normal function arguments.

That means ClangFormat currently incorrectly inserts whitespace
between `NS_SWIFT_NAME` arguments with colons and dots, so:

  extern UIWindow *MainWindow(void) NS_SWIFT_NAME(getter:MyHelper.mainWindow());

becomes:

  extern UIWindow *MainWindow(void) NS_SWIFT_NAME(getter : 
MyHelper.mainWindow());

which clang treats as a parser error:

  error: 'swift_name' attribute has invalid identifier for context name 
[-Werror,-Wswift-name-attribute]

Thankfully, D82620  recently added the ability 
to treat specific macros
as "whitespace sensitive", meaning their arguments are implicitly
treated as strings (so whitespace is not added anywhere inside).

This diff adds `NS_SWIFT_NAME` and `CF_SWIFT_NAME` to
`WhitespaceSensitiveMacros` so their arguments are implicitly treated
as whitespace-sensitive.

Test Plan:

  New tests added. Ran tests with:
  % ninja FormatTests && ./tools/clang/unittests/Format/FormatTests


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D89425

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1024,6 +1024,12 @@
   verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;");
   verifyFormat("@property(assign, getter=isEditable) BOOL editable;");
 
+  verifyFormat("extern UIWindow *MainWindow(void) "
+   "NS_SWIFT_NAME(getter:MyHelper.mainWindow());");
+
+  verifyFormat("extern UIWindow *MainWindow(void) "
+   "CF_SWIFT_NAME(getter:MyHelper.mainWindow());");
+
   Style.ColumnLimit = 50;
   verifyFormat("@interface Foo\n"
"- (void)doStuffWithFoo:(id)name\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -964,6 +964,8 @@
   LLVMStyle.WhitespaceSensitiveMacros.push_back("STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("PP_STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("BOOST_PP_STRINGIZE");
+  LLVMStyle.WhitespaceSensitiveMacros.push_back("NS_SWIFT_NAME");
+  LLVMStyle.WhitespaceSensitiveMacros.push_back("CF_SWIFT_NAME");
 
   // Defaults that differ when not C++.
   if (Language == FormatStyle::LK_TableGen) {


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -1024,6 +1024,12 @@
   verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;");
   verifyFormat("@property(assign, getter=isEditable) BOOL editable;");
 
+  verifyFormat("extern UIWindow *MainWindow(void) "
+   "NS_SWIFT_NAME(getter:MyHelper.mainWindow());");
+
+  verifyFormat("extern UIWindow *MainWindow(void) "
+   "CF_SWIFT_NAME(getter:MyHelper.mainWindow());");
+
   Style.ColumnLimit = 50;
   verifyFormat("@interface Foo\n"
"- (void)doStuffWithFoo:(id)name\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -964,6 +964,8 @@
   LLVMStyle.WhitespaceSensitiveMacros.push_back("STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("PP_STRINGIZE");
   LLVMStyle.WhitespaceSensitiveMacros.push_back("BOOST_PP_STRINGIZE");
+  LLVMStyle.WhitespaceSensitiveMacros.push_back("NS_SWIFT_NAME");
+  LLVMStyle.WhitespaceSensitiveMacros.push_back("CF_SWIFT_NAME");
 
   // Defaults that differ when not C++.
   if (Language == FormatStyle::LK_TableGen) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77571: Add ClangTidy check to find calls to NSInvocation methods under ARC that don't have proper object argument lifetimes.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added inline comments.
This revision is now accepted and ready to land.



Comment at: 
clang-tools-extra/clang-tidy/objc/NsinvocationArgumentLifetimeCheck.cpp:71-73
+  // Currently there is no way to directly get the source range for the
+  // __weak/__strong ObjC lifetime qualifiers, so it's necessary to string
+  // search in the source code.

Is there a bug filed for this? Seems like an oversight.




Comment at: 
clang-tools-extra/clang-tidy/objc/NsinvocationArgumentLifetimeCheck.cpp:116
+  auto Diag = diag(MatchedExpr->getArg(0)->getBeginLoc(),
+   "NSInvocation's %0 should only pass pointers to "
+   "objects with ownership __unsafe_unretained")

-[NSInvocation %0] should



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77571



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

[PATCH] D72876: Create a clang-tidy check to warn when -dealloc is implemented inside an ObjC class category.

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG0151ddc2e834: Create a clang-tidy check to warn when 
-dealloc is implemented inside an ObjC… (authored by mwyman, committed by 
benhamilton).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72876

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/DeallocInCategoryCheck.cpp
  clang-tools-extra/clang-tidy/objc/DeallocInCategoryCheck.h
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/objc-dealloc-in-category.rst
  clang-tools-extra/test/clang-tidy/checkers/objc-dealloc-in-category.m

Index: clang-tools-extra/test/clang-tidy/checkers/objc-dealloc-in-category.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/objc-dealloc-in-category.m
@@ -0,0 +1,47 @@
+// RUN: %check_clang_tidy %s objc-dealloc-in-category %t
+
+@interface NSObject
+// Used to quash warning about missing base class.
+- (void)dealloc;
+@end
+
+@interface Foo : NSObject
+@end
+
+@implementation Foo
+- (void)dealloc {
+  // No warning should be generated here.
+}
+@end
+
+@interface Bar : NSObject
+@end
+
+@interface Bar (BarCategory)
+@end
+
+@implementation Bar (BarCategory)
++ (void)dealloc {
+  // Should not trigger on class methods.
+}
+
+- (void)dealloc {
+  // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: category 'BarCategory' should not implement -dealloc [objc-dealloc-in-category]
+}
+@end
+
+@interface Baz : NSObject
+@end
+
+@implementation Baz
+- (void)dealloc {
+  // Should not trigger on implementation in the class itself, even with
+  // it declared in the category (below).
+}
+@end
+
+@interface Baz (BazCategory)
+// A declaration in a category @interface does not by itself provide an
+// overriding implementation, and should not generate a warning.
+- (void)dealloc;
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/objc-dealloc-in-category.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-dealloc-in-category.rst
@@ -0,0 +1,13 @@
+.. title:: clang-tidy - objc-dealloc-in-category
+
+objc-dealloc-in-category
+
+
+Finds implementations of ``-dealloc`` in Objective-C categories. The category
+implementation will override any ``-dealloc`` in the class implementation,
+potentially causing issues.
+
+Classes implement ``-dealloc`` to perform important actions to deallocate
+an object. If a category on the class implements ``-dealloc``, it will
+override the class's implementation and unexpected deallocation behavior
+may occur.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -232,6 +232,7 @@
`mpi-buffer-deref `_, "Yes"
`mpi-type-mismatch `_, "Yes"
`objc-avoid-nserror-init `_,
+   `objc-dealloc-in-category `_,
`objc-forbidden-subclassing `_,
`objc-missing-hash `_,
`objc-property-declaration `_, "Yes"
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -81,13 +81,18 @@
   ` check.
 
   Checks for usages of identifiers reserved for use by the implementation.
-  
+
 - New :doc:`cert-oop57-cpp
   ` check.
-  
+
   Flags use of the `C` standard library functions ``memset``, ``memcpy`` and
   ``memcmp`` and similar derivatives on non-trivial types.
 
+- New :doc:`objc-dealloc-in-category
+  ` check.
+
+  Finds implementations of -dealloc in Objective-C categories.
+
 New check aliases
 ^
 
@@ -111,8 +116,8 @@
 
 - Improved :doc:`readability-redundant-string-init
   ` check now supports a
-  `StringNames` option enabling its application to custom string classes. The 
-  check now detects in class initializers and constructor initializers which 
+  `StringNames` option enabling its application to custom string classes. The
+  check now detects in class initializers and constructor initializers which
   are deemed to be redundant.
 
 Renamed checks
Index: clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
===
--- clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
+++ clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
@@ -10,6 +10,7 @@
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "AvoidNSErrorInitCheck.h"
+#include "DeallocInCategoryCheck.h"
 #include "ForbiddenSubclassingCheck.h"
 #include "MissingHashCheck.h"
 #include "P

[PATCH] D72876: Create a clang-tidy check to warn when -dealloc is implemented inside an ObjC class category.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added a comment.

LGTM, just one nit-pick.




Comment at: clang-tools-extra/clang-tidy/objc/DeallocInCategoryCheck.h:24
+/// http://clang.llvm.org/extra/clang-tidy/checks/objc-dealloc-in-category.html
+class DeallocInCategoryCheck : public ClangTidyCheck {
+public:

`class DeallocInCategoryCheck final`



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72876



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

[PATCH] D67567: New ClangTidy check to warn when storing dispatch_once_t in non-static, non-global storage

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

It definitely shouldn't be in an `osx` directory since it's available on iOS 
and other Darwin-like operating systems.

Probably `darwin` would be OK. Adding a new subdirectory isn't trivial, there's 
a lot of places to update last I looked.

Should we tackle moving things around in a separate diff?


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

https://reviews.llvm.org/D67567



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

[PATCH] D65012: Adds support for formatting NS_CLOSED_ENUM and CF_CLOSED_ENUM alongside NS_ENUM and CF_ENUM.

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366719: Adds support for formatting NS_CLOSED_ENUM and 
CF_CLOSED_ENUM alongside NS_ENUM… (authored by benhamilton, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D65012?vs=211137&id=211153#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D65012

Files:
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/FormatToken.h
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp
  cfe/trunk/unittests/Format/FormatTestObjC.cpp

Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1716,6 +1716,8 @@
 
 TEST_F(FormatTest, FormatsNSEnums) {
   verifyGoogleFormat("typedef NS_ENUM(NSInteger, SomeName) { AAA, BBB }");
+  verifyGoogleFormat(
+  "typedef NS_CLOSED_ENUM(NSInteger, SomeName) { AAA, BBB }");
   verifyGoogleFormat("typedef NS_ENUM(NSInteger, MyType) {\n"
  "  // Information about someDecentlyLongValue.\n"
  "  someDecentlyLongValue,\n"
@@ -1724,6 +1726,14 @@
  "  // Information about aThirdDecentlyLongValue.\n"
  "  aThirdDecentlyLongValue\n"
  "};");
+  verifyGoogleFormat("typedef NS_CLOSED_ENUM(NSInteger, MyType) {\n"
+ "  // Information about someDecentlyLongValue.\n"
+ "  someDecentlyLongValue,\n"
+ "  // Information about anotherDecentlyLongValue.\n"
+ "  anotherDecentlyLongValue,\n"
+ "  // Information about aThirdDecentlyLongValue.\n"
+ "  aThirdDecentlyLongValue\n"
+ "};");
   verifyGoogleFormat("typedef NS_OPTIONS(NSInteger, MyType) {\n"
  "  a = 1,\n"
  "  b = 2,\n"
@@ -1734,6 +1744,11 @@
  "  b = 2,\n"
  "  c = 3,\n"
  "};");
+  verifyGoogleFormat("typedef CF_CLOSED_ENUM(NSInteger, MyType) {\n"
+ "  a = 1,\n"
+ "  b = 2,\n"
+ "  c = 3,\n"
+ "};");
   verifyGoogleFormat("typedef CF_OPTIONS(NSInteger, MyType) {\n"
  "  a = 1,\n"
  "  b = 2,\n"
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -114,7 +114,12 @@
   EXPECT_EQ(FormatStyle::LK_Cpp, Style->Language);
 
   Style =
-  getStyle("{}", "a.h", "none", "typedef NS_ENUM(NSInteger, Foo) {};\n");
+  getStyle("{}", "a.h", "none", "typedef NS_ENUM(int, Foo) {};\n");
+  ASSERT_TRUE((bool)Style);
+  EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
+
+  Style = getStyle("{}", "a.h", "none",
+   "typedef NS_CLOSED_ENUM(int, Foo) {};\n");
   ASSERT_TRUE((bool)Style);
   EXPECT_EQ(FormatStyle::LK_ObjC, Style->Language);
 
Index: cfe/trunk/lib/Format/FormatToken.h
===
--- cfe/trunk/lib/Format/FormatToken.h
+++ cfe/trunk/lib/Format/FormatToken.h
@@ -677,8 +677,10 @@
 kw_override = &IdentTable.get("override");
 kw_in = &IdentTable.get("in");
 kw_of = &IdentTable.get("of");
+kw_CF_CLOSED_ENUM = &IdentTable.get("CF_CLOSED_ENUM");
 kw_CF_ENUM = &IdentTable.get("CF_ENUM");
 kw_CF_OPTIONS = &IdentTable.get("CF_OPTIONS");
+kw_NS_CLOSED_ENUM = &IdentTable.get("NS_CLOSED_ENUM");
 kw_NS_ENUM = &IdentTable.get("NS_ENUM");
 kw_NS_OPTIONS = &IdentTable.get("NS_OPTIONS");
 
@@ -787,8 +789,10 @@
   IdentifierInfo *kw_override;
   IdentifierInfo *kw_in;
   IdentifierInfo *kw_of;
+  IdentifierInfo *kw_CF_CLOSED_ENUM;
   IdentifierInfo *kw_CF_ENUM;
   IdentifierInfo *kw_CF_OPTIONS;
+  IdentifierInfo *kw_NS_CLOSED_ENUM;
   IdentifierInfo *kw_NS_ENUM;
   IdentifierInfo *kw_NS_OPTIONS;
   IdentifierInfo *kw___except;
Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1215,7 +1215,8 @@
 case tok::kw_typedef:
   nextToken();
   if (FormatTok->isOneOf(Keywords.kw_NS_ENUM, Keywords.kw_NS_OPTIONS,
- Keywords.kw_CF_ENUM, Keywords.kw_CF_OPTIONS))
+ Keywords.kw_CF_ENUM, Keywords.kw_CF_OPTIONS,
+ Keywords.kw_CF_CLOSED_ENUM, Keywords.kw_NS_CLOSED_ENUM))
 parseEnum();
   break;
 case tok::kw_struct:
Index: cfe/trunk/l

[PATCH] D65012: Adds support for formatting NS_CLOSED_ENUM and CF_CLOSED_ENUM alongside NS_ENUM and CF_ENUM.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

Oh, I forgot there's one more place you should touch — the Objective-C style 
guesser for headers will need to be updated to understand that `NS_CLOSED_ENUM` 
indicates Objective-C:

https://reviews.llvm.org/source/llvm-github/browse/master/clang/lib/Format/Format.cpp$1688

Can you update this as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65012



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

[PATCH] D65012: Adds support for formatting NS_CLOSED_ENUM and CF_CLOSED_ENUM alongside NS_ENUM and CF_ENUM.

[Ben Hamilton via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65012



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

[PATCH] D65012: Adds support for formatting NS_CLOSED_ENUM alongside NS_ENUM.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton requested changes to this revision.
benhamilton added a comment.
This revision now requires changes to proceed.

Thanks! Don't forget the CF version.




Comment at: clang/lib/Format/FormatToken.h:793
   IdentifierInfo *kw_CF_OPTIONS;
+  IdentifierInfo *kw_NS_CLOSED_ENUM;
   IdentifierInfo *kw_NS_ENUM;

We'll also need `CF_CLOSED_ENUM`.




Comment at: clang/lib/Format/UnwrappedLineParser.cpp:1219
+ Keywords.kw_CF_ENUM, Keywords.kw_CF_OPTIONS,
+ Keywords.kw_NS_CLOSED_ENUM))
 parseEnum();

`Keywords.CF_CLOSED_ENUM` here as well.




Comment at: clang/unittests/Format/FormatTest.cpp:1717-1736
 TEST_F(FormatTest, FormatsNSEnums) {
   verifyGoogleFormat("typedef NS_ENUM(NSInteger, SomeName) { AAA, BBB }");
+  verifyGoogleFormat(
+  "typedef NS_CLOSED_ENUM(NSInteger, SomeName) { AAA, BBB }");
   verifyGoogleFormat("typedef NS_ENUM(NSInteger, MyType) {\n"
  "  // Information about someDecentlyLongValue.\n"
  "  someDecentlyLongValue,\n"

Please also add a test for `CF_CLOSED_ENUM`.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65012



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

[PATCH] D64775: [Format/ObjC] Avoid breaking between unary operators and operands

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL366592: [Format/ObjC] Avoid breaking between unary operators 
and operands (authored by benhamilton, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D64775?vs=210095&id=210859#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D64775

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


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2429,6 +2429,8 @@
   if (Left.is(TT_JavaAnnotation))
 return 50;
 
+  if (Left.is(TT_UnaryOperator))
+return 60;
   if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
   Left.Previous->isLabelString() &&
   (Left.NextOperator || Left.OperatorIndex != 0))
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -886,6 +886,18 @@
" bb:42\n"
" cc:42];");
 
+  // Avoid breaking between unary operators and ObjC method expressions.
+  Style.ColumnLimit = 45;
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "![foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "+[foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "-[foo bar]) {\n"
+   "}");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -2429,6 +2429,8 @@
   if (Left.is(TT_JavaAnnotation))
 return 50;
 
+  if (Left.is(TT_UnaryOperator))
+return 60;
   if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
   Left.Previous->isLabelString() &&
   (Left.NextOperator || Left.OperatorIndex != 0))
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -886,6 +886,18 @@
" bb:42\n"
" cc:42];");
 
+  // Avoid breaking between unary operators and ObjC method expressions.
+  Style.ColumnLimit = 45;
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "![foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "+[foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "-[foo bar]) {\n"
+   "}");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64775: [Format/ObjC] Avoid breaking between unary operators and ObjC method invocations

[Ben Hamilton via Phabricator via cfe-commits]

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

Any more thoughts on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64775



___
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.

Submitted as r366267. Thanks!




Comment at: clang/lib/Format/TokenAnnotator.cpp:389
   bool isCpp11AttributeSpecifier(const FormatToken &Tok) {
 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] Don't 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&id=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 &Tok) {
 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 &Tok) {
 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] D64775: [Format/ObjC] Avoid breaking between unary operators and ObjC method invocations

[Ben Hamilton via Phabricator via cfe-commits]

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



Comment at: clang/lib/Format/TokenAnnotator.cpp:2427-2428
 return 50;
 
+  if (Left.is(TT_UnaryOperator) && Right.is(TT_ObjCMethodExpr))
+return 60;

sammccall wrote:
> This looks a little suspicious only because it's so specific.
> 
> It seems like you'd *always* want a harsh penalty for breaking between a 
> unary operator and its operand. @klimek does this look right, should we drop 
> the `ObcJMethodExpr` requirement or is this case handled elsewhere?
I tried changing it to just `if (Left.is(TT_UnaryOperator))` and all the tests 
passed. I'll go for the gusto and assume this is fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64775



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

[PATCH] D64775: [Format/ObjC] Avoid breaking between unary operators and ObjC method invocations

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 210095.
benhamilton marked an inline comment as done.
benhamilton added a comment.

- Rebase.
- Change to just avoid breaking when the left-hand side is a unary operator


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64775

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


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -886,6 +886,18 @@
" bb:42\n"
" cc:42];");
 
+  // Avoid breaking between unary operators and ObjC method expressions.
+  Style.ColumnLimit = 45;
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "![foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "+[foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "-[foo bar]) {\n"
+   "}");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2425,6 +2425,8 @@
   if (Left.is(TT_JavaAnnotation))
 return 50;
 
+  if (Left.is(TT_UnaryOperator))
+return 60;
   if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
   Left.Previous->isLabelString() &&
   (Left.NextOperator || Left.OperatorIndex != 0))


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -886,6 +886,18 @@
" bb:42\n"
" cc:42];");
 
+  // Avoid breaking between unary operators and ObjC method expressions.
+  Style.ColumnLimit = 45;
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "![foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "+[foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "-[foo bar]) {\n"
+   "}");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2425,6 +2425,8 @@
   if (Left.is(TT_JavaAnnotation))
 return 50;
 
+  if (Left.is(TT_UnaryOperator))
+return 60;
   if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
   Left.Previous->isLabelString() &&
   (Left.NextOperator || Left.OperatorIndex != 0))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D64775: [Format/ObjC] Avoid breaking between unary operators and ObjC method invocations

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: krasimir, djasper, sammccall.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Test Plan:

  New tests added. Ran tests with:
  % ninja FormatTests && ./tools/clang/unittests/Format/FormatTests
  Confirmed tests failed before change and passed after change.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D64775

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


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -886,6 +886,18 @@
" bb:42\n"
" cc:42];");
 
+  // Avoid breaking between unary operators and ObjC method expressions.
+  Style.ColumnLimit = 45;
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "![foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "+[foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "-[foo bar]) {\n"
+   "}");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2425,6 +2425,8 @@
   if (Left.is(TT_JavaAnnotation))
 return 50;
 
+  if (Left.is(TT_UnaryOperator) && Right.is(TT_ObjCMethodExpr))
+return 60;
   if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
   Left.Previous->isLabelString() &&
   (Left.NextOperator || Left.OperatorIndex != 0))


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -886,6 +886,18 @@
" bb:42\n"
" cc:42];");
 
+  // Avoid breaking between unary operators and ObjC method expressions.
+  Style.ColumnLimit = 45;
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "![foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "+[foo bar]) {\n"
+   "}");
+  verifyFormat("if (a012345678901234567890123 &&\n"
+   "-[foo bar]) {\n"
+   "}");
+
   Style.ColumnLimit = 70;
   verifyFormat(
   "void f() {\n"
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2425,6 +2425,8 @@
   if (Left.is(TT_JavaAnnotation))
 return 50;
 
+  if (Left.is(TT_UnaryOperator) && Right.is(TT_ObjCMethodExpr))
+return 60;
   if (Left.isOneOf(tok::plus, tok::comma) && Left.Previous &&
   Left.Previous->isLabelString() &&
   (Left.NextOperator || Left.OperatorIndex != 0))
___
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

[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] D62045: Modified global variable declaration to fit updated objc guide.

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton requested changes to this revision.
benhamilton added inline comments.
This revision now requires changes to proceed.



Comment at: 
clang-tools-extra/clang-tidy/google/GlobalVariableDeclarationCheck.cpp:51-52
+
+  auto NewName = "g" + llvm::StringRef(std::string(1, FC)).upper()) +
+   Decl->getName().substr(1).str());
+

I don't see any guidance in the style guide about prefixing with `g` either. I 
assume we should remove that?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D62045



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

[PATCH] D60920: [ASTMatchers] Introduce Objective-C matchers `isClassMessage`, `isClassMethod`, and `isInstanceMethod`

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC358904: [ASTMatchers] Introduce Objective-C matchers 
`isClassMessage`, `isClassMethod`… (authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D60920?vs=195905&id=196095#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D60920

Files:
  docs/LibASTMatchersReference.html
  include/clang/ASTMatchers/ASTMatchers.h
  lib/ASTMatchers/Dynamic/Registry.cpp
  unittests/ASTMatchers/ASTMatchersTraversalTest.cpp

Index: lib/ASTMatchers/Dynamic/Registry.cpp
===
--- lib/ASTMatchers/Dynamic/Registry.cpp
+++ lib/ASTMatchers/Dynamic/Registry.cpp
@@ -344,6 +344,8 @@
   REGISTER_MATCHER(isBitField);
   REGISTER_MATCHER(isCatchAll);
   REGISTER_MATCHER(isClass);
+  REGISTER_MATCHER(isClassMessage);
+  REGISTER_MATCHER(isClassMethod);
   REGISTER_MATCHER(isConst);
   REGISTER_MATCHER(isConstQualified);
   REGISTER_MATCHER(isConstexpr);
@@ -367,6 +369,7 @@
   REGISTER_MATCHER(isInTemplateInstantiation);
   REGISTER_MATCHER(isInline);
   REGISTER_MATCHER(isInstanceMessage);
+  REGISTER_MATCHER(isInstanceMethod);
   REGISTER_MATCHER(isInstantiated);
   REGISTER_MATCHER(isInstantiationDependent);
   REGISTER_MATCHER(isInteger);
Index: unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
===
--- unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -454,6 +454,20 @@
   objcMessageExpr(hasReceiver(declRefExpr(to(varDecl(hasName("x";
 }
 
+TEST(Matcher, isClassMessage) {
+  EXPECT_TRUE(matchesObjC(
+  "@interface NSString +(NSString *) stringWithFormat; @end "
+  "void f() { [NSString stringWithFormat]; }",
+  objcMessageExpr(isClassMessage(;
+
+  EXPECT_FALSE(matchesObjC(
+  "@interface NSString @end "
+  "void f(NSString *x) {"
+  "[x containsString];"
+  "}",
+  objcMessageExpr(isClassMessage(;
+}
+
 TEST(Matcher, isInstanceMessage) {
   EXPECT_TRUE(matchesObjC(
   "@interface NSString @end "
@@ -469,6 +483,46 @@
 
 }
 
+TEST(Matcher, isClassMethod) {
+  EXPECT_TRUE(matchesObjC(
+"@interface Bar + (void)bar; @end",
+objcMethodDecl(isClassMethod(;
+
+  EXPECT_TRUE(matchesObjC(
+"@interface Bar @end"
+"@implementation Bar + (void)bar {} @end",
+objcMethodDecl(isClassMethod(;
+
+  EXPECT_FALSE(matchesObjC(
+"@interface Foo - (void)foo; @end",
+objcMethodDecl(isClassMethod(;
+
+  EXPECT_FALSE(matchesObjC(
+"@interface Foo @end "
+"@implementation Foo - (void)foo {} @end",
+objcMethodDecl(isClassMethod(;
+}
+
+TEST(Matcher, isInstanceMethod) {
+  EXPECT_TRUE(matchesObjC(
+"@interface Foo - (void)foo; @end",
+objcMethodDecl(isInstanceMethod(;
+
+  EXPECT_TRUE(matchesObjC(
+"@interface Foo @end "
+"@implementation Foo - (void)foo {} @end",
+objcMethodDecl(isInstanceMethod(;
+
+  EXPECT_FALSE(matchesObjC(
+"@interface Bar + (void)bar; @end",
+objcMethodDecl(isInstanceMethod(;
+
+  EXPECT_FALSE(matchesObjC(
+"@interface Bar @end"
+"@implementation Bar + (void)bar {} @end",
+objcMethodDecl(isInstanceMethod(;
+}
+
 TEST(MatcherCXXMemberCallExpr, On) {
   auto Snippet1 = R"cc(
 struct Y {
Index: docs/LibASTMatchersReference.html
===
--- docs/LibASTMatchersReference.html
+++ docs/LibASTMatchersReference.html
@@ -3567,11 +3567,24 @@
 
 
 
+MatcherObjCMessageExpr>isClassMessage
+Returns true when the Objective-C message is sent to a class.
+
+Example
+matcher = objcMessageExpr(isClassMessage())
+matches
+  [NSString stringWithFormat:@"format"];
+but not
+  NSString *x = @"hello";
+  [x containsString:@"h"];
+
+
+
 MatcherObjCMessageExpr>isInstanceMessage
 Returns true when the Objective-C message is sent to an instance.
 
 Example
-matcher = objcMessagaeExpr(isInstanceMessage())
+matcher = objcMessageExpr(isInstanceMessage())
 matches
   NSString *x = @"hello";
   [x containsString:@"h"];
@@ -3580,6 +3593,30 @@
 
 
 
+MatcherObjCMethodDecl>isClassMethod
+Returns true when the Objective-C method declaration is a class method.
+
+Example
+matcher = objcMethodDecl(isClassMethod())
+matches
+  @interface I + (void)foo; @end
+but not
+  @interface I - (void)bar; @end
+
+
+
+MatcherObjCMethodDecl>isInstanceMethod
+Returns true when the Objective-C method declaration is an instance method.
+
+Example
+matcher = objcMethodDecl(isInstanceMethod())
+matches
+  @interface I

[PATCH] D60920: [ASTMatchers] Introduce Objective-C matchers `isClassMessage`, `isClassMethod`, and `isInstanceMethod`

[Ben Hamilton via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rC Clang

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

https://reviews.llvm.org/D60920



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

[PATCH] D57923: [Format/ObjC] Fix [foo bar]->baz formatting as lambda arrow

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC353531: [Format/ObjC] Fix [foo bar]->baz formatting as 
lambda arrow (authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57923?vs=185859&id=185975#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57923

Files:
  lib/Format/TokenAnnotator.cpp
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,10 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr && CurrentToken->Next &&
+CurrentToken->Next->is(TT_LambdaArrow))
+  CurrentToken->Next->Type = TT_Unknown;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,10 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr && CurrentToken->Next &&
+CurrentToken->Next->is(TT_LambdaArrow))
+  CurrentToken->Next->Type = TT_Unknown;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57923: [Format/ObjC] Fix [foo bar]->baz formatting as lambda arrow

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 185859.
benhamilton added a comment.

Tidy up code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57923

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,10 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr && CurrentToken->Next &&
+CurrentToken->Next->is(TT_LambdaArrow))
+  CurrentToken->Next->Type = TT_Unknown;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,10 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr && CurrentToken->Next &&
+CurrentToken->Next->is(TT_LambdaArrow))
+  CurrentToken->Next->Type = TT_Unknown;
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57923: [Format/ObjC] Fix [foo bar]->baz formatting as lambda arrow

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: krasimir, djasper.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently, `UnwrappedLineParser` thinks an arrow token after
an ObjC method expression is a C++ lambda arrow, so it formats:

  [foo bar]->baz

as:

  [foo bar] -> baz

Because `UnwrappedLineParser` runs before `TokenAnnotator`, it can't
know if the arrow token is after an ObjC method expression or not.

This diff makes `TokenAnnotator` remove the TT_LambdaArrow on
the arrow token if it follows an ObjC method expression.

Test Plan: New test added. Ran test with:

  % ninja FormatTests && ./tools/clang/unittests/Format/FormatTests
  Confirmed test failed before diff and passed after diff.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57923

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTestObjC.cpp


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,11 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr) {
+  if (CurrentToken->Next && CurrentToken->Next->is(TT_LambdaArrow))
+CurrentToken->Next->Type = TT_Unknown;
+}
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does


Index: clang/unittests/Format/FormatTestObjC.cpp
===
--- clang/unittests/Format/FormatTestObjC.cpp
+++ clang/unittests/Format/FormatTestObjC.cpp
@@ -611,6 +611,7 @@
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
   verifyFormat("[foo bar:baz];");
+  verifyFormat("[foo bar]->baz;");
   verifyFormat("return [foo bar:baz];");
   verifyFormat("return (a)[foo bar:baz];");
   verifyFormat("f([foo bar:baz]);");
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1426,6 +1426,9 @@
   nextToken();
   break;
 case tok::arrow:
+  // This might or might not actually be a lambda arrow (this could be an
+  // ObjC method invocation followed by a dereferencing arrow). We might
+  // reset this back to TT_Unknown in TokenAnnotator.
   FormatTok->Type = TT_LambdaArrow;
   nextToken();
   break;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -520,6 +520,11 @@
   if (Parent && Parent->is(TT_PointerOrReference))
 Parent->Type = TT_BinaryOperator;
 }
+// An arrow after an ObjC method expression is not a lambda arrow.
+if (CurrentToken->Type == TT_ObjCMethodExpr) {
+  if (CurrentToken->Next && CurrentToken->Next->is(TT_LambdaArrow))
+CurrentToken->Next->Type = TT_Unknown;
+}
 Left->MatchingParen = CurrentToken;
 CurrentToken->MatchingParen = Left;
 // FirstObjCSelectorName is set when a colon is found. This does
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57623: [please ignore, v2] Clang test revision for new Herald rules for Github monorepo

[Ben Hamilton via Phabricator via cfe-commits]

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

This is another test for the new Herald rules to subscribe cfe-commits
to revisions sent out from the Github monorepo under the "clang/" subdirectory.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D57623

Files:
  clang/README.txt


Index: clang/README.txt
===
--- clang/README.txt
+++ clang/README.txt
@@ -25,3 +25,4 @@
 If you find a bug in Clang, please file it in the LLVM bug tracker:
   http://llvm.org/bugs/
 
+This is another test for the new Herald rules for the Github monorepo, please 
ignore.


Index: clang/README.txt
===
--- clang/README.txt
+++ clang/README.txt
@@ -25,3 +25,4 @@
 If you find a bug in Clang, please file it in the LLVM bug tracker:
   http://llvm.org/bugs/
 
+This is another test for the new Herald rules for the Github monorepo, please ignore.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57513: [please ignore] Clang test revision for new Herald rules for Github monorepo

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 184499.
benhamilton added a comment.
Herald added a subscriber: cfe-commits.
Herald added a project: clang.

- Another update


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57513

Files:
  clang/README.txt


Index: clang/README.txt
===
--- clang/README.txt
+++ clang/README.txt
@@ -25,3 +25,4 @@
 If you find a bug in Clang, please file it in the LLVM bug tracker:
   http://llvm.org/bugs/
 
+This is another test for the new Herald rules for the Github monorepo, please 
ignore.


Index: clang/README.txt
===
--- clang/README.txt
+++ clang/README.txt
@@ -25,3 +25,4 @@
 If you find a bug in Clang, please file it in the LLVM bug tracker:
   http://llvm.org/bugs/
 
+This is another test for the new Herald rules for the Github monorepo, please ignore.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56909: [clang-format] Fix line parsing for noexcept lambdas

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL352622: [clang-format] Fix line parsing for noexcept lambdas 
(authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56909?vs=182514&id=184281#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56909

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


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1422,6 +1422,7 @@
 case tok::numeric_constant:
 case tok::coloncolon:
 case tok::kw_mutable:
+case tok::kw_noexcept:
   nextToken();
   break;
 case tok::arrow:
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11725,6 +11725,8 @@
 
 TEST_F(FormatTest, FormatsLambdas) {
   verifyFormat("int c = [b]() mutable { return [&b] { return b++; }(); 
}();\n");
+  verifyFormat(
+  "int c = [b]() mutable noexcept { return [&b] { return b++; }(); 
}();\n");
   verifyFormat("int c = [&] { [=] { return b++; }(); }();\n");
   verifyFormat("int c = [&, &a, a] { [=, c, &d] { return b++; }(); }();\n");
   verifyFormat("int c = [&a, &a, a] { [=, a, b, &c] { return b++; }(); 
}();\n");


Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1422,6 +1422,7 @@
 case tok::numeric_constant:
 case tok::coloncolon:
 case tok::kw_mutable:
+case tok::kw_noexcept:
   nextToken();
   break;
 case tok::arrow:
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11725,6 +11725,8 @@
 
 TEST_F(FormatTest, FormatsLambdas) {
   verifyFormat("int c = [b]() mutable { return [&b] { return b++; }(); }();\n");
+  verifyFormat(
+  "int c = [b]() mutable noexcept { return [&b] { return b++; }(); }();\n");
   verifyFormat("int c = [&] { [=] { return b++; }(); }();\n");
   verifyFormat("int c = [&, &a, a] { [=, c, &d] { return b++; }(); }();\n");
   verifyFormat("int c = [&a, &a, a] { [=, a, b, &c] { return b++; }(); }();\n");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D56909: [clang-format] Fix line parsing for noexcept lambdas

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.
Herald added a subscriber: llvm-commits.

So sorry for the delay. I've landed this diff as rC352622 
.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56909



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

[PATCH] D56226: [clang-format] square parens that are followed by '=' are not Objective-C message sends

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:501
+CurrentToken->Next->is(tok::equal))) ||
+  (CurrentToken->Previous->Previous == Left)) &&
  Left->is(TT_ObjCMethodExpr)) {

I think you need to check if `CurrentToken->Previous` is null before 
dereferencing it.




Comment at: lib/Format/TokenAnnotator.cpp:504
+  // An ObjC method call is rarely followed by an open parenthesis or
+  // an assignment. It also usually contains more than one token.
   // FIXME: Do we incorrectly label ":" with this?

I think it's actually required to have more than one token. Can we reduce the 
change to that?


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

https://reviews.llvm.org/D56226



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

[PATCH] D55545: Allow IncludeSorter to use #import for Objective-C files

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: clang-tidy/utils/IncludeSorter.cpp:116
   bool IsAngled) {
+  std::string IncludeDirective = LangOpts->ObjC ? "#import " : "#include ";
   std::string IncludeStmt =

hokein wrote:
> What about the ObjC++? The current behavior is always using `#import`, google 
> objc style guide says `#import Objective-C and Objective-C++ headers, and 
> #include C/C++ headers.`, I don't think we have a way to do it smartly.
> 
> Maybe a conservative way is to use `#import ` for ObjC only?
Yeah, came here to say this. Let's enable `#import` for ObjC but disable for 
C++ (which will include ObjC++).



Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55545



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

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:115
   diag(MatchedDecl->getLocation(),
-   "function name %0 not using function naming conventions described by "
-   "Google Objective-C style guide")
-  << MatchedDecl << generateFixItHint(MatchedDecl);
+   "%select{static|global}1 function name %0 must %select{be in|have an "
+   "appropriate prefix followed by}1 Pascal case as required by Google "

stephanemoore wrote:
> aaron.ballman wrote:
> > stephanemoore wrote:
> > > benhamilton wrote:
> > > > I know "global" is the correct name, but I feel like "non-static" might 
> > > > be clearer to folks dealing with these error messages.
> > > > 
> > > > WDYT?
> > > > 
> > > I'm wary of saying "non-static" because namespaced functions in 
> > > Objective-C++ are not subject to the cited rules. The term "non-static" 
> > > seems like it could be interpreted to extend to namespaced functions 
> > > which could potentially mislead someone into adding prefixes to 
> > > namespaced functions in Objective-C++.
> > How about "%select{static function|function in global namespace}1 named 
> > %0..."
> Definitely better.
Yeah, this is better. Thanks.


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

https://reviews.llvm.org/D55482



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

[PATCH] D55482: [clang-tidy] Improve google-objc-function-naming diagnostics 📙

[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, just minor suggestions.




Comment at: clang-tidy/google/FunctionNamingCheck.cpp:114-118
   diag(MatchedDecl->getLocation(),
-   "function name %0 not using function naming conventions described by "
-   "Google Objective-C style guide")
-  << MatchedDecl << generateFixItHint(MatchedDecl);
+   "%select{static|global}1 function name %0 must %select{be in|have an "
+   "appropriate prefix followed by}1 Pascal case as required by Google "
+   "Objective-C style guide")
+  << MatchedDecl << IsGlobal << generateFixItHint(MatchedDecl);

Should we suggest making the function static when it fails this check? (I 
assume the vast majority of functions which fail this check should really be 
static.)



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:115
   diag(MatchedDecl->getLocation(),
-   "function name %0 not using function naming conventions described by "
-   "Google Objective-C style guide")
-  << MatchedDecl << generateFixItHint(MatchedDecl);
+   "%select{static|global}1 function name %0 must %select{be in|have an "
+   "appropriate prefix followed by}1 Pascal case as required by Google "

I know "global" is the correct name, but I feel like "non-static" might be 
clearer to folks dealing with these error messages.

WDYT?



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

https://reviews.llvm.org/D55482



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

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

> Would you be okay with landing this fix and if we get any further reports for 
> Objective-C++ sources then we can suppress it in all C++/Objective-C++ 
> sources? I think there is enough value to enforcing the naming conventions on 
> non-namespaced C functions in Objective-C++ to justify a simple followup fix. 
> If other issues are reported after this then I also agree that enforcement in 
> Objective-C++ sources may incur more overhead than it's worth.

I'm not against it, but we've already disabled the majority of Objective-C 
checks for Objective-C++ code, so I don't think this one should apply either.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55101



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

[PATCH] D55101: [clang-tidy] Ignore namespaced and C++ member functions in google-objc-function-naming check 🙈

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

IMHO we should just disable this check entirely for C++ files (including 
Objective-C++).

Since Objective-C++ files will have a mix of the Google Objective-C and Google 
C++ styles, I think there will be too many places where we'll hit conflicts 
like this (there are likely more).


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D55101



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

[PATCH] D51575: [clang-tidy/checks] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: docs/clang-tidy/checks/google-objc-function-naming.rst:20
+  static bool is_positive(int i) { return i > 0; }
+  bool IsNegative(int i) { return i < 0; }
+

This is not actually handled by the check, right? (You even have a test which 
confirms this.)

As far as I can tell, this check essentially looks for at least one capital 
letter and no `_` in function names, right?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D54111: [clang-format] Do not threat the asm clobber [ as ObjCExpr

[Ben Hamilton via Phabricator via cfe-commits]

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

In the diff description, please fix the typo: `Do not threat the asm clobber` 
-> `Do not treat the asm clobber`




Comment at: unittests/Format/FormatTest.cpp:12756-12763
+TEST_F(FormatTest, GuessedLanguageWithInlineAsmClobbers) {
+  EXPECT_EQ(FormatStyle::LK_Cpp, guessLanguage("foo.h",
+   "void f() {\n"
+   "  asm (\"mov %[e], %[d]\"\n"
+   " : [d] \"=rm\" (d)\n"
+   "   [e] \"rm\" (*e));\n"
+   "}"));

Worth a second test with `asm volatile (...)` ?



Repository:
  rC Clang

https://reviews.llvm.org/D54111



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

[PATCH] D54110: [Format] Add debugging to ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC346144: [Format] Add debugging to ObjC language guesser 
(authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D54110?vs=172599&id=172600#toc

Repository:
  rC Clang

https://reviews.llvm.org/D54110

Files:
  lib/Format/Format.cpp


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1504,16 +1504,19 @@
   SmallVectorImpl &AnnotatedLines,
   FormatTokenLexer &Tokens) override {
 assert(Style.Language == FormatStyle::LK_Cpp);
-IsObjC = guessIsObjC(AnnotatedLines, Tokens.getKeywords());
+IsObjC = guessIsObjC(Env.getSourceManager(), AnnotatedLines,
+ Tokens.getKeywords());
 tooling::Replacements Result;
 return {Result, 0};
   }
 
   bool isObjC() { return IsObjC; }
 
 private:
-  static bool guessIsObjC(const SmallVectorImpl 
&AnnotatedLines,
-  const AdditionalKeywords &Keywords) {
+  static bool
+  guessIsObjC(const SourceManager &SourceManager,
+  const SmallVectorImpl &AnnotatedLines,
+  const AdditionalKeywords &Keywords) {
 // Keep this array sorted, since we are binary searching over it.
 static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
 "CGFloat",
@@ -1604,9 +1607,15 @@
TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
+  LLVM_DEBUG(llvm::dbgs()
+ << "Detected ObjC at location "
+ << FormatTok->Tok.getLocation().printToString(
+SourceManager)
+ << " token: " << FormatTok->TokenText << " token type: "
+ << getTokenTypeName(FormatTok->Type) << "\n");
   return true;
 }
-if (guessIsObjC(Line->Children, Keywords))
+if (guessIsObjC(SourceManager, Line->Children, Keywords))
   return true;
   }
 }


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1504,16 +1504,19 @@
   SmallVectorImpl &AnnotatedLines,
   FormatTokenLexer &Tokens) override {
 assert(Style.Language == FormatStyle::LK_Cpp);
-IsObjC = guessIsObjC(AnnotatedLines, Tokens.getKeywords());
+IsObjC = guessIsObjC(Env.getSourceManager(), AnnotatedLines,
+ Tokens.getKeywords());
 tooling::Replacements Result;
 return {Result, 0};
   }
 
   bool isObjC() { return IsObjC; }
 
 private:
-  static bool guessIsObjC(const SmallVectorImpl &AnnotatedLines,
-  const AdditionalKeywords &Keywords) {
+  static bool
+  guessIsObjC(const SourceManager &SourceManager,
+  const SmallVectorImpl &AnnotatedLines,
+  const AdditionalKeywords &Keywords) {
 // Keep this array sorted, since we are binary searching over it.
 static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
 "CGFloat",
@@ -1604,9 +1607,15 @@
TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
+  LLVM_DEBUG(llvm::dbgs()
+ << "Detected ObjC at location "
+ << FormatTok->Tok.getLocation().printToString(
+SourceManager)
+ << " token: " << FormatTok->TokenText << " token type: "
+ << getTokenTypeName(FormatTok->Type) << "\n");
   return true;
 }
-if (guessIsObjC(Line->Children, Keywords))
+if (guessIsObjC(SourceManager, Line->Children, Keywords))
   return true;
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D54110: [Format] Add debugging to ObjC language guesser

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added a reviewer: krasimir.
Herald added a subscriber: cfe-commits.

To handle diagnosing bugs where ObjCHeaderStyleGuesser guesses
wrong, this diff adds a bit more debug logging to the Objective-C
language guesser.


Repository:
  rC Clang

https://reviews.llvm.org/D54110

Files:
  lib/Format/Format.cpp


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1504,16 +1504,19 @@
   SmallVectorImpl &AnnotatedLines,
   FormatTokenLexer &Tokens) override {
 assert(Style.Language == FormatStyle::LK_Cpp);
-IsObjC = guessIsObjC(AnnotatedLines, Tokens.getKeywords());
+IsObjC = guessIsObjC(Env.getSourceManager(), AnnotatedLines,
+ Tokens.getKeywords());
 tooling::Replacements Result;
 return {Result, 0};
   }
 
   bool isObjC() { return IsObjC; }
 
 private:
-  static bool guessIsObjC(const SmallVectorImpl 
&AnnotatedLines,
-  const AdditionalKeywords &Keywords) {
+  static bool
+  guessIsObjC(const SourceManager &SourceManager,
+  const SmallVectorImpl &AnnotatedLines,
+  const AdditionalKeywords &Keywords) {
 // Keep this array sorted, since we are binary searching over it.
 static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
 "CGFloat",
@@ -1604,9 +1607,15 @@
TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
+  LLVM_DEBUG(llvm::dbgs()
+ << "Detected ObjC at location "
+ << FormatTok->Tok.getLocation().printToString(
+SourceManager)
+ << " token: " << FormatTok->TokenText << " token type: "
+ << getTokenTypeName(FormatTok->Type) << "\n");
   return true;
 }
-if (guessIsObjC(Line->Children, Keywords))
+if (guessIsObjC(SourceManager, Line->Children, Keywords))
   return true;
   }
 }


Index: lib/Format/Format.cpp
===
--- lib/Format/Format.cpp
+++ lib/Format/Format.cpp
@@ -1504,16 +1504,19 @@
   SmallVectorImpl &AnnotatedLines,
   FormatTokenLexer &Tokens) override {
 assert(Style.Language == FormatStyle::LK_Cpp);
-IsObjC = guessIsObjC(AnnotatedLines, Tokens.getKeywords());
+IsObjC = guessIsObjC(Env.getSourceManager(), AnnotatedLines,
+ Tokens.getKeywords());
 tooling::Replacements Result;
 return {Result, 0};
   }
 
   bool isObjC() { return IsObjC; }
 
 private:
-  static bool guessIsObjC(const SmallVectorImpl &AnnotatedLines,
-  const AdditionalKeywords &Keywords) {
+  static bool
+  guessIsObjC(const SourceManager &SourceManager,
+  const SmallVectorImpl &AnnotatedLines,
+  const AdditionalKeywords &Keywords) {
 // Keep this array sorted, since we are binary searching over it.
 static constexpr llvm::StringLiteral FoundationIdentifiers[] = {
 "CGFloat",
@@ -1604,9 +1607,15 @@
TT_ObjCBlockLBrace, TT_ObjCBlockLParen,
TT_ObjCDecl, TT_ObjCForIn, TT_ObjCMethodExpr,
TT_ObjCMethodSpecifier, TT_ObjCProperty)) {
+  LLVM_DEBUG(llvm::dbgs()
+ << "Detected ObjC at location "
+ << FormatTok->Tok.getLocation().printToString(
+SourceManager)
+ << " token: " << FormatTok->TokenText << " token type: "
+ << getTokenTypeName(FormatTok->Type) << "\n");
   return true;
 }
-if (guessIsObjC(Line->Children, Keywords))
+if (guessIsObjC(SourceManager, Line->Children, Keywords))
   return true;
   }
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53197: [clang-format] Fix BraceWrapping AfterFunction for ObjC methods

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

> @benhamilton Could you land this patch?

Done. Thanks for your contribution!


Repository:
  rC Clang

https://reviews.llvm.org/D53197



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

[PATCH] D53197: [clang-format] Fix BraceWrapping AfterFunction for ObjC methods

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC344406: [clang-format] Fix BraceWrapping AfterFunction for 
ObjC methods (authored by benhamilton, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D53197

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTestObjC.cpp


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2164,6 +2164,8 @@
   addUnwrappedLine();
   return;
 } else if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterFunction)
+addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
   addUnwrappedLine();
   return;
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -584,6 +584,16 @@
" a:(a)yyy\n"
"   bbb:(d);");
   verifyFormat("- (void)drawRectOn:(id)surface 
ofSize:(aaa)height:(bbb)width;");
+
+  // BraceWrapping AfterFunction is respected for ObjC methods 
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  verifyFormat("@implementation Foo\n"
+   "- (void)foo:(id)bar\n"
+   "{\n"
+   "}\n"
+   "@end\n");
 }
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2164,6 +2164,8 @@
   addUnwrappedLine();
   return;
 } else if (FormatTok->Tok.is(tok::l_brace)) {
+  if (Style.BraceWrapping.AfterFunction)
+addUnwrappedLine();
   parseBlock(/*MustBeDeclaration=*/false);
   addUnwrappedLine();
   return;
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -584,6 +584,16 @@
" a:(a)yyy\n"
"   bbb:(d);");
   verifyFormat("- (void)drawRectOn:(id)surface ofSize:(aaa)height:(bbb)width;");
+
+  // BraceWrapping AfterFunction is respected for ObjC methods 
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterFunction = true;
+  verifyFormat("@implementation Foo\n"
+   "- (void)foo:(id)bar\n"
+   "{\n"
+   "}\n"
+   "@end\n");
 }
 
 TEST_F(FormatTestObjC, FormatObjCMethodExpr) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D53197: [clang-format] Fix BraceWrapping AfterFunction for ObjC methods

[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:
  rC Clang

https://reviews.llvm.org/D53197



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

[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧

[Ben Hamilton via Phabricator via cfe-commits]

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

This is fine, but please update the comments (and docs?) to make it clear that 
we no longer enforce camelCase but allow aRBiTraRYcAsE now.




Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:28-31
 // For StandardProperty the naming style is 'lowerCamelCase'.
 // For CategoryProperty especially in categories of system class,
 // to avoid naming conflict, the suggested naming style is
 // 'abc_lowerCamelCase' (adding lowercase prefix followed by '_').

These comments are no longer accurate.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:73-74
+  //
+  // Disallow names of this form:
+  // LongString
   std::string StartMatcher = UsedInMatcher ? "::" : "^";

Just to be clear, this also allows things like aRbITRaRyCapS, right? We should 
comment that this is explicitly by design.



Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:113-115
   // the property name should be in Lower Camel Case like
   // 'lowerCamelCase'
+  unless(matchesName(validPropertyNameRegex(true

This comment is no longer accurate.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51832



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

[PATCH] D51819: [clang-tidy/ObjC] Update list of acronyms in PropertyDeclarationCheck

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341720: [clang-tidy/ObjC] Update list of acronyms in 
PropertyDeclarationCheck (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51819?vs=164523&id=164525#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51819

Files:
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -42,12 +42,15 @@
 "[2-9]G",
 "ACL",
 "API",
+"APN",
+"APNS",
 "AR",
 "ARGB",
 "ASCII",
 "AV",
 "BGRA",
 "CA",
+"CDN",
 "CF",
 "CG",
 "CI",
@@ -71,14 +74,17 @@
 "ID",
 "JPG",
 "JS",
+"JSON",
 "LAN",
 "LZW",
+"LTR",
 "MAC",
 "MD",
 "MDNS",
 "MIDI",
 "NS",
 "OS",
+"P2P",
 "PDF",
 "PIN",
 "PNG",
@@ -102,12 +108,14 @@
 "SSO",
 "TCP",
 "TIFF",
+"TOS",
 "TTS",
 "UI",
 "URI",
 "URL",
 "UUID",
 "VC",
+"VO",
 "VOIP",
 "VPN",
 "VR",


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -42,12 +42,15 @@
 "[2-9]G",
 "ACL",
 "API",
+"APN",
+"APNS",
 "AR",
 "ARGB",
 "ASCII",
 "AV",
 "BGRA",
 "CA",
+"CDN",
 "CF",
 "CG",
 "CI",
@@ -71,14 +74,17 @@
 "ID",
 "JPG",
 "JS",
+"JSON",
 "LAN",
 "LZW",
+"LTR",
 "MAC",
 "MD",
 "MDNS",
 "MIDI",
 "NS",
 "OS",
+"P2P",
 "PDF",
 "PIN",
 "PNG",
@@ -102,12 +108,14 @@
 "SSO",
 "TCP",
 "TIFF",
+"TOS",
 "TTS",
 "UI",
 "URI",
 "URL",
 "UUID",
 "VC",
+"VO",
 "VOIP",
 "VPN",
 "VR",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51819: [clang-tidy/ObjC] Update list of acronyms in PropertyDeclarationCheck

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 164523.
benhamilton added a comment.

- Update docs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51819

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp
  docs/clang-tidy/checks/objc-property-declaration.rst


Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -63,7 +63,7 @@
If set to ``1``, the value in ``Acronyms`` is appended to the
default list of acronyms:
 
-   
``ACL;API;ARGB;ASCII;BGRA;CMYK;DNS;FPS;FTP;GIF;GPS;HD;HDR;HTML;HTTP;HTTPS;HUD;ID;JPG;JS;LAN;LZW;MDNS;MIDI;OS;PDF;PIN;PNG;POI;PSTN;PTR;QA;QOS;RGB;RGBA;RGBX;ROM;RPC;RTF;RTL;SDK;SSO;TCP;TIFF;TTS;UI;URI;URL;VC;VOIP;VPN;VR;WAN;XML``.
+   
``[2-9]G;ACL;API;APN;APNS;AR;ARGB;ASCII;AV;BGRA;CA;CDN;CF;CG;CI;CRC;CV;CMYK;DNS;FPS;FTP;GIF;GL;GPS;GUID;HD;HDR;HMAC;HTML;HTTP;HTTPS;HUD;ID;JPG;JS;JSON;LAN;LZW;LTR;MAC;MD;MDNS;MIDI;NS;OS;P2P;PDF;PIN;PNG;POI;PSTN;PTR;QA;QOS;RGB;RGBA;RGBX;RIPEMD;ROM;RPC;RTF;RTL;SC;SDK;SHA;SQL;SSO;TCP;TIFF;TOS;TTS;UI;URI;URL;UUID;VC;VO;VOIP;VPN;VR;W;WAN;X;XML;Y;Z``.
 
If set to ``0``, the value in ``Acronyms`` replaces the default list
of acronyms.
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -42,12 +42,15 @@
 "[2-9]G",
 "ACL",
 "API",
+"APN",
+"APNS",
 "AR",
 "ARGB",
 "ASCII",
 "AV",
 "BGRA",
 "CA",
+"CDN",
 "CF",
 "CG",
 "CI",
@@ -71,14 +74,17 @@
 "ID",
 "JPG",
 "JS",
+"JSON",
 "LAN",
 "LZW",
+"LTR",
 "MAC",
 "MD",
 "MDNS",
 "MIDI",
 "NS",
 "OS",
+"P2P",
 "PDF",
 "PIN",
 "PNG",
@@ -102,12 +108,14 @@
 "SSO",
 "TCP",
 "TIFF",
+"TOS",
 "TTS",
 "UI",
 "URI",
 "URL",
 "UUID",
 "VC",
+"VO",
 "VOIP",
 "VPN",
 "VR",


Index: docs/clang-tidy/checks/objc-property-declaration.rst
===
--- docs/clang-tidy/checks/objc-property-declaration.rst
+++ docs/clang-tidy/checks/objc-property-declaration.rst
@@ -63,7 +63,7 @@
If set to ``1``, the value in ``Acronyms`` is appended to the
default list of acronyms:
 
-   ``ACL;API;ARGB;ASCII;BGRA;CMYK;DNS;FPS;FTP;GIF;GPS;HD;HDR;HTML;HTTP;HTTPS;HUD;ID;JPG;JS;LAN;LZW;MDNS;MIDI;OS;PDF;PIN;PNG;POI;PSTN;PTR;QA;QOS;RGB;RGBA;RGBX;ROM;RPC;RTF;RTL;SDK;SSO;TCP;TIFF;TTS;UI;URI;URL;VC;VOIP;VPN;VR;WAN;XML``.
+   ``[2-9]G;ACL;API;APN;APNS;AR;ARGB;ASCII;AV;BGRA;CA;CDN;CF;CG;CI;CRC;CV;CMYK;DNS;FPS;FTP;GIF;GL;GPS;GUID;HD;HDR;HMAC;HTML;HTTP;HTTPS;HUD;ID;JPG;JS;JSON;LAN;LZW;LTR;MAC;MD;MDNS;MIDI;NS;OS;P2P;PDF;PIN;PNG;POI;PSTN;PTR;QA;QOS;RGB;RGBA;RGBX;RIPEMD;ROM;RPC;RTF;RTL;SC;SDK;SHA;SQL;SSO;TCP;TIFF;TOS;TTS;UI;URI;URL;UUID;VC;VO;VOIP;VPN;VR;W;WAN;X;XML;Y;Z``.
 
If set to ``0``, the value in ``Acronyms`` replaces the default list
of acronyms.
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -42,12 +42,15 @@
 "[2-9]G",
 "ACL",
 "API",
+"APN",
+"APNS",
 "AR",
 "ARGB",
 "ASCII",
 "AV",
 "BGRA",
 "CA",
+"CDN",
 "CF",
 "CG",
 "CI",
@@ -71,14 +74,17 @@
 "ID",
 "JPG",
 "JS",
+"JSON",
 "LAN",
 "LZW",
+"LTR",
 "MAC",
 "MD",
 "MDNS",
 "MIDI",
 "NS",
 "OS",
+"P2P",
 "PDF",
 "PIN",
 "PNG",
@@ -102,12 +108,14 @@
 "SSO",
 "TCP",
 "TIFF",
+"TOS",
 "TTS",
 "UI",
 "URI",
 "URL",
 "UUID",
 "VC",
+"VO",
 "VOIP",
 "VPN",
 "VR",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51819: [clang-tidy/ObjC] Update list of acronyms in PropertyDeclarationCheck

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: Wizard, hokein.
Herald added a subscriber: cfe-commits.

This adds a few common acronyms we found were missing from 
PropertyDeclarationCheck.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51819

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -42,12 +42,15 @@
 "[2-9]G",
 "ACL",
 "API",
+"APN",
+"APNS",
 "AR",
 "ARGB",
 "ASCII",
 "AV",
 "BGRA",
 "CA",
+"CDN",
 "CF",
 "CG",
 "CI",
@@ -71,14 +74,17 @@
 "ID",
 "JPG",
 "JS",
+"JSON",
 "LAN",
 "LZW",
+"LTR",
 "MAC",
 "MD",
 "MDNS",
 "MIDI",
 "NS",
 "OS",
+"P2P",
 "PDF",
 "PIN",
 "PNG",
@@ -102,12 +108,14 @@
 "SSO",
 "TCP",
 "TIFF",
+"TOS",
 "TTS",
 "UI",
 "URI",
 "URL",
 "UUID",
 "VC",
+"VO",
 "VOIP",
 "VPN",
 "VR",


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -42,12 +42,15 @@
 "[2-9]G",
 "ACL",
 "API",
+"APN",
+"APNS",
 "AR",
 "ARGB",
 "ASCII",
 "AV",
 "BGRA",
 "CA",
+"CDN",
 "CF",
 "CG",
 "CI",
@@ -71,14 +74,17 @@
 "ID",
 "JPG",
 "JS",
+"JSON",
 "LAN",
 "LZW",
+"LTR",
 "MAC",
 "MD",
 "MDNS",
 "MIDI",
 "NS",
 "OS",
+"P2P",
 "PDF",
 "PIN",
 "PNG",
@@ -102,12 +108,14 @@
 "SSO",
 "TCP",
 "TIFF",
+"TOS",
 "TTS",
 "UI",
 "URI",
 "URL",
 "UUID",
 "VC",
+"VO",
 "VOIP",
 "VPN",
 "VR",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:35
+  // non-standard capitalized character sequences including acronyms,
+  // initialisms, and prefixes of symbols (e.g., UIColorFromNSString). For this
+  // reason, the regex only verifies that the function name after the prefix

benhamilton wrote:
> Any reason why this is different from the implementation in the property name 
> checker? Either we should allow both of:
> 
> ```
> void ARBiTraRilyNameDFuncTioN();
> // ...
> @property (...) id arBItrArIlyNameD;
> ```
> 
> or we should require that acronyms in the middle of the name be 
> registered/known acronyms for both properties and functions.
> 
> I believe this diff allows arbitrary capitalization for functions, but we 
> disallowed that for property names, so I think we should be consistent.
(And, just to be clear, I don't feel strongly which direction we go — it's 
certainly less work to allow arbitrarily-named functions and properties than to 
maintain the acronym list.)



Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton requested changes to this revision.
benhamilton added a comment.
This revision now requires changes to proceed.

Thanks for this! Let's consolidate this with the property name checker (either 
simplify the logic there and allow `arBiTRAryCapSAnYWHere` or apply the same 
registered acronym logic here).




Comment at: clang-tidy/google/FunctionNamingCheck.cpp:35
+  // non-standard capitalized character sequences including acronyms,
+  // initialisms, and prefixes of symbols (e.g., UIColorFromNSString). For this
+  // reason, the regex only verifies that the function name after the prefix

Any reason why this is different from the implementation in the property name 
checker? Either we should allow both of:

```
void ARBiTraRilyNameDFuncTioN();
// ...
@property (...) id arBItrArIlyNameD;
```

or we should require that acronyms in the middle of the name be 
registered/known acronyms for both properties and functions.

I believe this diff allows arbitrary capitalization for functions, but we 
disallowed that for property names, so I think we should be consistent.



Comment at: clang-tidy/google/FunctionNamingCheck.cpp:50
+
+void FunctionNamingCheck::registerMatchers(MatchFinder *Finder) {
+  // This check should only be applied to Objective-C sources.

Wizard wrote:
> Can we do some simple check to see if some easy fix can be provided just like 
> `objc-property-declaration` check?
> Something like `static bool isPositive` to `static bool IsPositive` and 
> `static bool is_upper_camel` to `IsUpperCamel`. Such check can help provide 
> code fix for a lot of  very common mistake at a low cost (i.e. if the naming 
> pattern cannot be simply recognized, just provide no fix).
+1, I think the two checks should be substantially similar.



Comment at: clang-tidy/google/FunctionNamingCheck.h:21
+
+/// Finds function names that do not conform to the recommendations of the
+/// Google Objective-C Style Guide. Function names should be in upper camel 
case

Worth mentioning this does not apply to Objective-C method names, nor 
Objective-C properties.



Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51575



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

[PATCH] D49190: [clang-tidy/ObjC] Add SQL to list of acronyms

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE336919: [clang-tidy/ObjC] Add SQL to list of acronyms 
(authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D49190?vs=155004&id=155216#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49190

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -98,6 +98,7 @@
 "SC",
 "SDK",
 "SHA",
+"SQL",
 "SSO",
 "TCP",
 "TIFF",


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -98,6 +98,7 @@
 "SC",
 "SDK",
 "SHA",
+"SQL",
 "SSO",
 "TCP",
 "TIFF",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D49190: [clang-tidy/ObjC] Add SQL to list of acronyms

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: Wizard, hokein.
Herald added a subscriber: cfe-commits.

SQL is a common acronym.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D49190

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -98,6 +98,7 @@
 "SC",
 "SDK",
 "SHA",
+"SQL",
 "SSO",
 "TCP",
 "TIFF",


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -98,6 +98,7 @@
 "SC",
 "SDK",
 "SHA",
+"SQL",
 "SSO",
 "TCP",
 "TIFF",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48718: [clang-format] Prohibit breaking after a bracket opening ObjC method expression

[Ben Hamilton via Phabricator via cfe-commits]

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

> Sorry for the confusion. If you prefer me to add this test and modify in 
> later commit I'll do it.

I see. Just mentioning what you did in the diff description is probably OK.


Repository:
  rC Clang

https://reviews.llvm.org/D48718



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

[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC

[Ben Hamilton via Phabricator via cfe-commits]

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

In https://reviews.llvm.org/D48716#1149293, @jolesiak wrote:

> General comment to changes https://reviews.llvm.org/D48716, 
> https://reviews.llvm.org/D48718, https://reviews.llvm.org/D48719, 
> https://reviews.llvm.org/D48720 (which are the split of 
> https://reviews.llvm.org/D48352):




> These changes are separate, in the sense that they fix different issues. 
> However, they should be chained as every change (apart from the first one) is 
> based on previous ones: https://reviews.llvm.org/D48716 -> 
> https://reviews.llvm.org/D48718 -> https://reviews.llvm.org/D48719 -> 
> https://reviews.llvm.org/D48720. I don't know how to chain them in 
> Phabricator (I should probably leave some comments about what every change is 
> based on though).

Phabricator has an `Edit Related Revisions` feature where you can tag other 
revisions as being dependencies of or depending upon the current revision:

F6560886: Screen Shot 2018-07-02 at 10.57.37 AM.png 


I went ahead and used it to link the revisions in the order you listed above. 
(I also like to edit the 1-line description and add `[1/x]` at the beginning — 
but that's up to you.)

> https://reviews.llvm.org/D48716 fixes the mechanism of counting parameters. 
> This is an internal change though and doesn't influence formatting on its own 
> (at the current state). Its lack would be visible after applying 
> https://reviews.llvm.org/D48719. Therefore, I'm not aware of any formatting 
> test that fails before applying this change and succeeds after. As far as I 
> know internal functions/methods are not tested at all. Thus, the tests are 
> part of https://reviews.llvm.org/D48719.

Great. Just mentioning that in the diff description should be fine.


Repository:
  rC Clang

https://reviews.llvm.org/D48716



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

[PATCH] D48720: [clang-format] Put ObjC method arguments into one line when they fit

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton accepted this revision.
benhamilton added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Format/ContinuationIndenter.cpp:1411
+  // line).
+  if (Current.MatchingParen && Current.MatchingParen->Previous) {
+const FormatToken &CurrentScopeOpener = *Current.MatchingParen->Previous;

Should we check if `State.Stack.back().BreakBeforeParameter` is `true` before 
doing any of this?


Repository:
  rC Clang

https://reviews.llvm.org/D48720



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

[PATCH] D48719: [clang-format] Fix split priorities for ObjC methods

[Ben Hamilton via Phabricator via cfe-commits]

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

Nice improvement!


Repository:
  rC Clang

https://reviews.llvm.org/D48719



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

[PATCH] D48679: [clang-format/ObjC] Fix NS_SWIFT_NAME(foo(bar:baz:)) after ObjC method decl

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL335983: [clang-format/ObjC] Fix NS_SWIFT_NAME(foo(bar:baz:)) 
after ObjC method decl (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D48679

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


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -698,13 +698,19 @@
  Line.startsWith(TT_ObjCMethodSpecifier)) {
 Tok->Type = TT_ObjCMethodExpr;
 const FormatToken *BeforePrevious = Tok->Previous->Previous;
+// Ensure we tag all identifiers in method declarations as
+// TT_SelectorName.
+bool UnknownIdentifierInMethodDeclaration =
+Line.startsWith(TT_ObjCMethodSpecifier) &&
+Tok->Previous->is(tok::identifier) && 
Tok->Previous->is(TT_Unknown);
 if (!BeforePrevious ||
 // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
 !(BeforePrevious->is(TT_CastRParen) ||
   (BeforePrevious->is(TT_ObjCMethodExpr) &&
BeforePrevious->is(tok::colon))) ||
 BeforePrevious->is(tok::r_square) ||
-Contexts.back().LongestObjCSelectorName == 0) {
+Contexts.back().LongestObjCSelectorName == 0 ||
+UnknownIdentifierInMethodDeclaration) {
   Tok->Previous->Type = TT_SelectorName;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -923,6 +923,14 @@
   verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;");
   verifyFormat("@property(assign, getter=isEditable) BOOL editable;");
 
+  Style.ColumnLimit = 50;
+  verifyFormat("@interface Foo\n"
+   "- (void)doStuffWithFoo:(id)name\n"
+   "   bar:(id)bar\n"
+   "   baz:(id)baz\n"
+   "NS_SWIFT_NAME(doStuff(withFoo:bar:baz:));\n"
+   "@end");
+
   Style = getMozillaStyle();
   verifyFormat("@property (assign, getter=isEditable) BOOL editable;");
   verifyFormat("@property BOOL editable;");


Index: cfe/trunk/lib/Format/TokenAnnotator.cpp
===
--- cfe/trunk/lib/Format/TokenAnnotator.cpp
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp
@@ -698,13 +698,19 @@
  Line.startsWith(TT_ObjCMethodSpecifier)) {
 Tok->Type = TT_ObjCMethodExpr;
 const FormatToken *BeforePrevious = Tok->Previous->Previous;
+// Ensure we tag all identifiers in method declarations as
+// TT_SelectorName.
+bool UnknownIdentifierInMethodDeclaration =
+Line.startsWith(TT_ObjCMethodSpecifier) &&
+Tok->Previous->is(tok::identifier) && Tok->Previous->is(TT_Unknown);
 if (!BeforePrevious ||
 // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
 !(BeforePrevious->is(TT_CastRParen) ||
   (BeforePrevious->is(TT_ObjCMethodExpr) &&
BeforePrevious->is(tok::colon))) ||
 BeforePrevious->is(tok::r_square) ||
-Contexts.back().LongestObjCSelectorName == 0) {
+Contexts.back().LongestObjCSelectorName == 0 ||
+UnknownIdentifierInMethodDeclaration) {
   Tok->Previous->Type = TT_SelectorName;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;
Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -923,6 +923,14 @@
   verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;");
   verifyFormat("@property(assign, getter=isEditable) BOOL editable;");
 
+  Style.ColumnLimit = 50;
+  verifyFormat("@interface Foo\n"
+   "- (void)doStuffWithFoo:(id)name\n"
+   "   bar:(id)bar\n"
+   "   baz:(id)baz\n"
+   "NS_SWIFT_NAME(doStuff(withFoo:bar:baz:));\n"
+   "@end");
+
   Style = getMozillaStyle();
   verifyFormat("@property (assign, getter=isEditable) BOOL editable;");
   verifyFormat("@property BOOL editable;");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48716: [clang-format] Fix counting parameters/arguments for ObjC

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton requested changes to this revision.
benhamilton added a comment.
This revision now requires changes to proceed.

> Count selector parts also for method declarations.

What bug does this fix? Can you add a test which breaks before this change and 
is fixed by this change?




Comment at: lib/Format/FormatToken.h:247-248
+  /// If this is the first ObjC selector name in an ObjC method
+  /// definition or call, this contains the number of parts that whole selector
+  /// consist of.
   unsigned ObjCSelectorNameParts = 0;

Grammar nit: that whole selector consist of -> that the whole selector consists 
of




Comment at: lib/Format/TokenAnnotator.cpp:519
+// FirstObjCSelectorName is set when a colon is found. This does
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is

Grammar nit-pick: when method -> when the method




Comment at: lib/Format/TokenAnnotator.cpp:520
+// not work, however, when method has no parameters.
+// Here, we set FirstObjCSelectorName when the end of the expression is
+// reached, in case it was not set already.

expression -> method call




Comment at: lib/Format/TokenAnnotator.cpp:627
   void updateParameterCount(FormatToken *Left, FormatToken *Current) {
+// For ObjC methods number of parameters is calculated differently as
+// method declarations have different structure (parameters are not inside

Grammar nit-pick: methods number -> methods, the number




Comment at: lib/Format/TokenAnnotator.cpp:628
+// For ObjC methods number of parameters is calculated differently as
+// method declarations have different structure (parameters are not inside
+// parenthesis scope).

Grammar nit-picks:

* have different structure -> have a different structure
* parameters are not -> the parameters are not




Comment at: lib/Format/TokenAnnotator.cpp:628-629
+// For ObjC methods number of parameters is calculated differently as
+// method declarations have different structure (parameters are not inside
+// parenthesis scope).
 if (Current->is(tok::l_brace) && Current->BlockKind == BK_Block)

benhamilton wrote:
> Grammar nit-picks:
> 
> * have different structure -> have a different structure
> * parameters are not -> the parameters are not
> 
Why does parenthesis scope matter here? `updateParameterCount()` is called from 
`parseSquare()`.

I'm not sure what the goal of this change is.



Repository:
  rC Clang

https://reviews.llvm.org/D48716



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

[PATCH] D48718: [clang-format] Prohibit breaking after a bracket opening ObjC method expression

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton requested changes to this revision.
benhamilton added a comment.
This revision now requires changes to proceed.

Can you add a test, please?


Repository:
  rC Clang

https://reviews.llvm.org/D48718



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

[PATCH] D48679: [clang-format/ObjC] Fix NS_SWIFT_NAME(foo(bar:baz:)) after ObjC method decl

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: djasper, krasimir.
Herald added a subscriber: cfe-commits.

In https://reviews.llvm.org/D44638, I partially fixed 
`NS_SWIFT_NAME(foo(bar:baz:))`-style
annotations on C functions, but didn't add a test for Objective-C
method declarations.

For ObjC method declarations which are annotated with `NS_SWIFT_NAME(...)`,
we currently fail to annotate the final component of the selector
name as `TT_SelectorName`.

Because the token type is left unknown, clang-format will happily
cause a compilation error when it changes the following:

  @interface Foo
  - (void)doStuffWithFoo:(id)name
 bar:(id)bar
 baz:(id)baz
  NS_SWIFT_NAME(doStuff(withFoo:bar:baz:));
  @end

to:

  @interface Foo
  - (void)doStuffWithFoo:(id)name
 bar:(id)bar
 baz:(id)baz
  NS_SWIFT_NAME(doStuff(withFoo:bar:baz
  :));
  @end

(note the linebreak before the final `:`).

The logic which decides whether or not to annotate the token before a
`:` with `TT_SelectorName` is pretty fragile, and has to handle some
pretty odd cases like pair-parameters:

  [I drawRectOn:surface ofSize:aa:bbb atOrigin:cc:dd];

So, to minimize the effect of this change, I decided to only annotate
unknown identifiers before a `:` as `TT_SelectorName` for Objective-C
declaration lines.

Test Plan: New tests included. Confirmed tests failed before change and

  passed after change. Ran tests with:
  % make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests


Repository:
  rC Clang

https://reviews.llvm.org/D48679

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


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -923,6 +923,14 @@
   verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;");
   verifyFormat("@property(assign, getter=isEditable) BOOL editable;");
 
+  Style.ColumnLimit = 50;
+  verifyFormat("@interface Foo\n"
+   "- (void)doStuffWithFoo:(id)name\n"
+   "   bar:(id)bar\n"
+   "   baz:(id)baz\n"
+   "NS_SWIFT_NAME(doStuff(withFoo:bar:baz:));\n"
+   "@end");
+
   Style = getMozillaStyle();
   verifyFormat("@property (assign, getter=isEditable) BOOL editable;");
   verifyFormat("@property BOOL editable;");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -698,13 +698,19 @@
  Line.startsWith(TT_ObjCMethodSpecifier)) {
 Tok->Type = TT_ObjCMethodExpr;
 const FormatToken *BeforePrevious = Tok->Previous->Previous;
+// Ensure we tag all identifiers in method declarations as
+// TT_SelectorName.
+bool UnknownIdentifierInMethodDeclaration =
+Line.startsWith(TT_ObjCMethodSpecifier) &&
+Tok->Previous->is(tok::identifier) && 
Tok->Previous->is(TT_Unknown);
 if (!BeforePrevious ||
 // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
 !(BeforePrevious->is(TT_CastRParen) ||
   (BeforePrevious->is(TT_ObjCMethodExpr) &&
BeforePrevious->is(tok::colon))) ||
 BeforePrevious->is(tok::r_square) ||
-Contexts.back().LongestObjCSelectorName == 0) {
+Contexts.back().LongestObjCSelectorName == 0 ||
+UnknownIdentifierInMethodDeclaration) {
   Tok->Previous->Type = TT_SelectorName;
   if (!Contexts.back().FirstObjCSelectorName)
 Contexts.back().FirstObjCSelectorName = Tok->Previous;


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -923,6 +923,14 @@
   verifyFormat("@property(assign, nonatomic) CGFloat hoverAlpha;");
   verifyFormat("@property(assign, getter=isEditable) BOOL editable;");
 
+  Style.ColumnLimit = 50;
+  verifyFormat("@interface Foo\n"
+   "- (void)doStuffWithFoo:(id)name\n"
+   "   bar:(id)bar\n"
+   "   baz:(id)baz\n"
+   "NS_SWIFT_NAME(doStuff(withFoo:bar:baz:));\n"
+   "@end");
+
   Style = getMozillaStyle();
   verifyFormat("@property (assign, getter=isEditable) BOOL editable;");
   verifyFormat("@property BOOL editable;");
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -698,13 +698,19 @@
  Line.startsWith(TT_ObjCMethodSpecifier)) {
 Tok->Type = TT_ObjCMethodExpr;
 const FormatToken *BeforePre

[PATCH] D48652: [clang-tidy/ObjC] Add hashing algorithm acronyms to objc-property-declaration

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL335770: [clang-tidy/ObjC] Add hashing algorithm acronyms to 
objc-property-declaration (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D48652

Files:
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -51,6 +51,7 @@
 "CF",
 "CG",
 "CI",
+"CRC",
 "CV",
 "CMYK",
 "DNS",
@@ -62,6 +63,7 @@
 "GUID",
 "HD",
 "HDR",
+"HMAC",
 "HTML",
 "HTTP",
 "HTTPS",
@@ -71,6 +73,8 @@
 "JS",
 "LAN",
 "LZW",
+"MAC",
+"MD",
 "MDNS",
 "MIDI",
 "NS",
@@ -86,12 +90,14 @@
 "RGB",
 "RGBA",
 "RGBX",
+"RIPEMD",
 "ROM",
 "RPC",
 "RTF",
 "RTL",
 "SC",
 "SDK",
+"SHA",
 "SSO",
 "TCP",
 "TIFF",


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -51,6 +51,7 @@
 "CF",
 "CG",
 "CI",
+"CRC",
 "CV",
 "CMYK",
 "DNS",
@@ -62,6 +63,7 @@
 "GUID",
 "HD",
 "HDR",
+"HMAC",
 "HTML",
 "HTTP",
 "HTTPS",
@@ -71,6 +73,8 @@
 "JS",
 "LAN",
 "LZW",
+"MAC",
+"MD",
 "MDNS",
 "MIDI",
 "NS",
@@ -86,12 +90,14 @@
 "RGB",
 "RGBA",
 "RGBX",
+"RIPEMD",
 "ROM",
 "RPC",
 "RTF",
 "RTL",
 "SC",
 "SDK",
+"SHA",
 "SSO",
 "TCP",
 "TIFF",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48352: [clang-format] Improve ObjC method expressions formatting

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton requested changes to this revision.
benhamilton added a comment.
This revision now requires changes to proceed.

It's really hard to understand reviews which change 4 different things.

I hate to ask, but can you split this up into the 4 fixes?


Repository:
  rC Clang

https://reviews.llvm.org/D48352



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

[PATCH] D48652: [clang-tidy/ObjC] Add hashing algorithm acronyms to objc-property-declaration

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added a reviewer: Wizard.
Herald added a subscriber: cfe-commits.

This PR adds a few acronyms related to hashing algorithms to the standard
list in `objc-property-declaration`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48652

Files:
  clang-tidy/objc/PropertyDeclarationCheck.cpp


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -51,6 +51,7 @@
 "CF",
 "CG",
 "CI",
+"CRC",
 "CV",
 "CMYK",
 "DNS",
@@ -62,6 +63,7 @@
 "GUID",
 "HD",
 "HDR",
+"HMAC",
 "HTML",
 "HTTP",
 "HTTPS",
@@ -71,6 +73,8 @@
 "JS",
 "LAN",
 "LZW",
+"MAC",
+"MD",
 "MDNS",
 "MIDI",
 "NS",
@@ -86,12 +90,14 @@
 "RGB",
 "RGBA",
 "RGBX",
+"RIPEMD",
 "ROM",
 "RPC",
 "RTF",
 "RTL",
 "SC",
 "SDK",
+"SHA",
 "SSO",
 "TCP",
 "TIFF",


Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -51,6 +51,7 @@
 "CF",
 "CG",
 "CI",
+"CRC",
 "CV",
 "CMYK",
 "DNS",
@@ -62,6 +63,7 @@
 "GUID",
 "HD",
 "HDR",
+"HMAC",
 "HTML",
 "HTTP",
 "HTTPS",
@@ -71,6 +73,8 @@
 "JS",
 "LAN",
 "LZW",
+"MAC",
+"MD",
 "MDNS",
 "MIDI",
 "NS",
@@ -86,12 +90,14 @@
 "RGB",
 "RGBA",
 "RGBX",
+"RIPEMD",
 "ROM",
 "RPC",
 "RTF",
 "RTL",
 "SC",
 "SDK",
+"SHA",
 "SSO",
 "TCP",
 "TIFF",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL334739: [clang-format] Disable 
AlwaysBreakBeforeMultilineStrings in Google style for… (authored by 
benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D47393?vs=149338&id=151379#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D47393

Files:
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/unittests/Format/FormatTestObjC.cpp


Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -1218,6 +1218,17 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.ColumnLimit = 40;
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -823,6 +823,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 


Index: cfe/trunk/unittests/Format/FormatTestObjC.cpp
===
--- cfe/trunk/unittests/Format/FormatTestObjC.cpp
+++ cfe/trunk/unittests/Format/FormatTestObjC.cpp
@@ -1218,6 +1218,17 @@
"}");
 }
 
+TEST_F(FormatTestObjC, AlwaysBreakBeforeMultilineStrings) {
+  Style = getGoogleStyle(FormatStyle::LK_ObjC);
+  Style.ColumnLimit = 40;
+  verifyFormat(" = @\"\"\n"
+   "   @\"\";");
+  verifyFormat("(@\"\"\n"
+   " @\"\");");
+  verifyFormat("(qqq, @\"\"\n"
+   "  @\"\");");
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: cfe/trunk/lib/Format/Format.cpp
===
--- cfe/trunk/lib/Format/Format.cpp
+++ cfe/trunk/lib/Format/Format.cpp
@@ -823,6 +823,7 @@
 // has been implemented.
 GoogleStyle.BreakStringLiterals = false;
   } else if (Language == FormatStyle::LK_ObjC) {
+GoogleStyle.AlwaysBreakBeforeMultilineStrings = false;
 GoogleStyle.ColumnLimit = 100;
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47393: [clang-format] Disable AlwaysBreakBeforeMultilineStrings in Google style for Objective-C 📜

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

I'll just land it for you.


Repository:
  rC Clang

https://reviews.llvm.org/D47393



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

[PATCH] D46922: [checks/property-decls] Fix comment in clang-tidy/objc/PropertyDeclarationCheck.cpp ✍️

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL334238: [checks/property-decls] Fix comment in 
clang-tidy/objc/PropertyDeclarationCheck. (authored by benhamilton, committed 
by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D46922?vs=146981&id=150409#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D46922

Files:
  clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -34,7 +34,7 @@
   CategoryProperty = 2,
 };
 
-/// The acronyms are from
+/// The acronyms are aggregated from multiple sources including
 /// 
https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
 ///
 /// Keep this list sorted.


Index: clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -34,7 +34,7 @@
   CategoryProperty = 2,
 };
 
-/// The acronyms are from
+/// The acronyms are aggregated from multiple sources including
 /// https://developer.apple.com/library/content/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
 ///
 /// Keep this list sorted.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46922: [checks/property-decls] Fix comment in clang-tidy/objc/PropertyDeclarationCheck.cpp ✍️

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

> Is it possible to get someone to land this for me? I don't believe I have 
> access to land it myself.

Done.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46922



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

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC333553: [clang-format/ObjC] Correctly parse Objective-C 
methods with 'class' in name (authored by benhamilton, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D47095?vs=148231&id=149124#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47095

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -328,7 +328,14 @@
"}\n"
"+ (id)init;\n"
"@end");
-
+  verifyFormat("@interface Foo\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end\n"
+   "@implementation Bar\n"
+   "- (void)bar {\n"
+   "}\n"
+   "@end");
   Style.ColumnLimit = 40;
   verifyFormat("@interface c () <\n"
"c, c,\n"
@@ -969,6 +976,59 @@
   verifyFormat("MACRO(new:)\n");
   verifyFormat("MACRO(delete:)\n");
   verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
+  verifyFormat("@implementation Foo\n"
+   "// Testing\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end\n");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+
+  verifyFormat("@interface Foo\n"
+   "// Testing\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end\n");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
 }
 
 TEST_F(FormatTestObjC, ObjCLiterals) {
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -120,6 +120,7 @@
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCMethod();
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();
   void parseObjCInterfaceOrImplementation();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2130,6 +2130,24 @@
   // "} n, m;" will end up in one unwrapped line.
 }
 
+void UnwrappedLineParser::parseObjCMethod() {
+  assert(FormatTok->Tok.isOneOf(tok::l_paren, tok::identifier) &&
+ "'(' or identifier expected.");
+  do {
+if (FormatTok->Tok.is(tok::semi)) {
+  nextToken();
+  addUnwrappedLine();
+  return;
+} else if (FormatTok->Tok.is(tok::l_brace)) {
+  parseBlock(/*MustBeDeclaration=*/false);
+  addUnwrappedLine();
+  return;
+} else {
+  nextToken();
+}
+  } while (!eof());
+}
+
 void UnwrappedLineParser::parseObjCProtocolList() {
   assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
   do {
@@ -2157,6 +2175,9 @@
   // Ignore stray "}". parseStructuralElement doesn't consume them.
   nextToken();
   addUnwrappedLine();
+} else if (FormatTok->isOneOf(tok::minus, tok::plus)) {
+  nextToken();
+  parseObjCMethod();
 } else {
   parseStructuralElement();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 148231.
benhamilton marked 2 inline comments as done.
benhamilton added a comment.

@jolesiak


Repository:
  rC Clang

https://reviews.llvm.org/D47095

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -328,7 +328,14 @@
"}\n"
"+ (id)init;\n"
"@end");
-
+  verifyFormat("@interface Foo\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end\n"
+   "@implementation Bar\n"
+   "- (void)bar {\n"
+   "}\n"
+   "@end");
   Style.ColumnLimit = 40;
   verifyFormat("@interface c () <\n"
"c, c,\n"
@@ -998,6 +1005,59 @@
   verifyFormat("MACRO(new:)\n");
   verifyFormat("MACRO(delete:)\n");
   verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
+  verifyFormat("@implementation Foo\n"
+   "// Testing\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end\n");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+
+  verifyFormat("@interface Foo\n"
+   "// Testing\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end\n");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
 }
 
 TEST_F(FormatTestObjC, ObjCLiterals) {
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -120,6 +120,7 @@
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCMethod();
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();
   void parseObjCInterfaceOrImplementation();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2130,6 +2130,24 @@
   // "} n, m;" will end up in one unwrapped line.
 }
 
+void UnwrappedLineParser::parseObjCMethod() {
+  assert(FormatTok->Tok.isOneOf(tok::l_paren, tok::identifier) &&
+ "'(' or identifier expected.");
+  do {
+if (FormatTok->Tok.is(tok::semi)) {
+  nextToken();
+  addUnwrappedLine();
+  return;
+} else if (FormatTok->Tok.is(tok::l_brace)) {
+  parseBlock(/*MustBeDeclaration=*/false);
+  addUnwrappedLine();
+  return;
+} else {
+  nextToken();
+}
+  } while (!eof());
+}
+
 void UnwrappedLineParser::parseObjCProtocolList() {
   assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
   do {
@@ -2157,6 +2175,9 @@
   // Ignore stray "}". parseStructuralElement doesn't consume them.
   nextToken();
   addUnwrappedLine();
+} else if (FormatTok->isOneOf(tok::minus, tok::plus)) {
+  nextToken();
+  parseObjCMethod();
 } else {
   parseStructuralElement();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: lib/Format/UnwrappedLineParser.cpp:2137
+  do {
+if (FormatTok->Tok.isOneOf(tok::semi, tok::r_brace)) {
+  nextToken();

jolesiak wrote:
> `tok::r_brace` could be skiped - see comment to line 2143.
Done.



Comment at: lib/Format/UnwrappedLineParser.cpp:2143
+  parseBlock(/*MustBeDeclaration=*/false);
+  addUnwrappedLine();
+} else {

jolesiak wrote:
> We have to add `return` after `addUnwrappedLine` as `parseBlock` does consume 
> `tok::r_brace`. Without `return` we will consume tokens after `}`. This 
> problem will rarely occur as most lines end with `tok::semi` or 
> `tok::r_brace` and it will be terminated properly (however maybe not handled 
> properly as we just skip every token in `else`) by `if` branch.
> 
> Test like:
> ```
> @implementation Foo
> - (foo)foo {
> }
> @end
> @implementation Bar
> - (bar)bar {
> }
> @end
> ```
> will distinguish version with `return` from one without. Therefore, I think 
> we should add it.
Done, test added.


Repository:
  rC Clang

https://reviews.llvm.org/D47095



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

[PATCH] D40221: [clang-format] Parse blocks in braced lists

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton edited reviewers, added: klimek; removed: djasper.
benhamilton added a comment.

@djasper isn't available to review.

At a high level, this seems good, but I'd like @klimek to take a look.




Comment at: lib/Format/UnwrappedLineParser.cpp:1320
+// \endcode
+bool UnwrappedLineParser::tryToParseBlock() {
+  // Consume the leading ^.

Is it standard to return a value from these `tryToParseFoo()` methods, even if 
nobody uses it?

I think we should either check the return value somewhere, or make this return 
`void`.



Comment at: lib/Format/UnwrappedLineParser.cpp:1324-1327
+  if (!Style.isCpp()) {
+// Blocks are only supported in C++ and Objective-C.
+return false;
+  }

Style: Remove curly braces for one-line if blocks.



https://reviews.llvm.org/D40221



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

[PATCH] D47195: [clang-format] Fix ObjC message arguments handling

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

Can you update the diff description to reflect that it includes the original 
change?


Repository:
  rC Clang

https://reviews.llvm.org/D47195



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

[PATCH] D47195: [clang-format] Fix ObjC message arguments handling

[Ben Hamilton via Phabricator via cfe-commits]

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

Please address @krasimir's comments, but looks good to me.


Repository:
  rC Clang

https://reviews.llvm.org/D47195



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

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 147611.
benhamilton added a comment.

Format


Repository:
  rC Clang

https://reviews.llvm.org/D47095

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -998,6 +998,59 @@
   verifyFormat("MACRO(new:)\n");
   verifyFormat("MACRO(delete:)\n");
   verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
+  verifyFormat("@implementation Foo\n"
+   "// Testing\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end\n");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+
+  verifyFormat("@interface Foo\n"
+   "// Testing\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end\n");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
 }
 
 TEST_F(FormatTestObjC, ObjCLiterals) {
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -120,6 +120,7 @@
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCMethod();
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();
   void parseObjCInterfaceOrImplementation();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2130,6 +2130,23 @@
   // "} n, m;" will end up in one unwrapped line.
 }
 
+void UnwrappedLineParser::parseObjCMethod() {
+  assert(FormatTok->Tok.isOneOf(tok::l_paren, tok::identifier) &&
+ "'(' or identifier expected.");
+  do {
+if (FormatTok->Tok.isOneOf(tok::semi, tok::r_brace)) {
+  nextToken();
+  addUnwrappedLine();
+  return;
+} else if (FormatTok->Tok.is(tok::l_brace)) {
+  parseBlock(/*MustBeDeclaration=*/false);
+  addUnwrappedLine();
+} else {
+  nextToken();
+}
+  } while (!eof());
+}
+
 void UnwrappedLineParser::parseObjCProtocolList() {
   assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
   do {
@@ -2157,6 +2174,9 @@
   // Ignore stray "}". parseStructuralElement doesn't consume them.
   nextToken();
   addUnwrappedLine();
+} else if (FormatTok->isOneOf(tok::minus, tok::plus)) {
+  nextToken();
+  parseObjCMethod();
 } else {
   parseStructuralElement();
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47095: [clang-format/ObjC] Correctly parse Objective-C methods with 'class' in name

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: djasper, jolesiak.
Herald added subscribers: cfe-commits, klimek.

Please take a close look at this CL. I haven't touched much of
`UnwrappedLineParser` before, so I may have gotten things wrong.

Previously, clang-format would incorrectly format the following:

  @implementation Foo
  
  - (Class)class {
  }
  
  - (void)foo {
  }
  
  @end

as:

  @implementation Foo
  
  - (Class)class {
  }
  
  - (void)foo {
  }
  
  @end

The problem is whenever `UnwrappedLineParser::parseStructuralElement()`
sees any of the keywords `class`, `struct`, or `enum`, it calls
`parseRecord()` to parse them as a C/C++ record.

This causes subsequent lines to be parsed incorrectly, which
causes them to be indented incorrectly.

In Objective-C/Objective-C++, these keywords are valid selector
components.

This diff fixes the issue by explicitly handling `+` and `-` lines
inside `@implementation` / `@interface` / `@protocol` blocks
and parsing them as Objective-C methods.

Test Plan: New tests added. Ran tests with:

  make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests


Repository:
  rC Clang

https://reviews.llvm.org/D47095

Files:
  lib/Format/UnwrappedLineParser.cpp
  lib/Format/UnwrappedLineParser.h
  unittests/Format/FormatTestObjC.cpp

Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -998,6 +998,59 @@
   verifyFormat("MACRO(new:)\n");
   verifyFormat("MACRO(delete:)\n");
   verifyFormat("foo = @{MACRO(new:) : MACRO(delete:)}\n");
+  verifyFormat("@implementation Foo\n"
+   "// Testing\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end\n");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "- (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+  verifyFormat("@implementation Foo\n"
+   "+ (Class)class:(Class)klass {\n"
+   "}\n"
+   "- (void)foo {\n"
+   "}\n"
+   "@end");
+
+  verifyFormat("@interface Foo\n"
+   "// Testing\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end\n");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "- (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
+  verifyFormat("@interface Foo\n"
+   "+ (Class)class:(Class)klass;\n"
+   "- (void)foo;\n"
+   "@end");
 }
 
 TEST_F(FormatTestObjC, ObjCLiterals) {
Index: lib/Format/UnwrappedLineParser.h
===
--- lib/Format/UnwrappedLineParser.h
+++ lib/Format/UnwrappedLineParser.h
@@ -120,6 +120,7 @@
   // parses the record as a child block, i.e. if the class declaration is an
   // expression.
   void parseRecord(bool ParseAsExpr = false);
+  void parseObjCMethod();
   void parseObjCProtocolList();
   void parseObjCUntilAtEnd();
   void parseObjCInterfaceOrImplementation();
Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -2130,6 +2130,22 @@
   // "} n, m;" will end up in one unwrapped line.
 }
 
+void UnwrappedLineParser::parseObjCMethod() {
+  assert(FormatTok->Tok.isOneOf(tok::l_paren, tok::identifier) && "'(' or identifier expected.");
+  do {
+if (FormatTok->Tok.isOneOf(tok::semi, tok::r_brace)) {
+  nextToken();
+  addUnwrappedLine();
+  return;
+} else if (FormatTok->Tok.is(tok::l_brace)) {
+  parseBlock(/*MustBeDeclaration=*/false);
+  addUnwrappedLine();
+} else {
+  nextToken();
+}
+  } while (!eof());
+}
+
 void UnwrappedLineParser::parseObjCProtocolList() {
   assert(FormatTok->Tok.is(tok::less) && "'<' expected.");
   do {
@@ -2157,6 +2173,9 @@
   // Ignore stray "}". parseStructuralElement doesn't consume them.
   nextToken();
   addUnwrappedLine();
+} else if (FormatTok->isOneOf(tok::minus, tok::plus)) {
+  nextToken();
+  parseObjCMethod();
 } else {
   parseStruct

[PATCH] D47028: [clang-format/ObjC] Correctly annotate single-component ObjC method invocations

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC332727: [clang-format/ObjC] Correctly annotate 
single-component ObjC method invocations (authored by benhamilton, committed by 
).

Changed prior to commit:
  https://reviews.llvm.org/D47028?vs=147516&id=147517#toc

Repository:
  rC Clang

https://reviews.llvm.org/D47028

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


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -501,6 +501,12 @@
 }
 if (StartsObjCMethodExpr && CurrentToken->Previous != Left) {
   CurrentToken->Type = TT_ObjCMethodExpr;
+  // If we haven't seen a colon yet, make sure the last identifier
+  // before the r_square is tagged as a selector name component.
+  if (!ColonFound && CurrentToken->Previous &&
+  CurrentToken->Previous->is(TT_Unknown) &&
+  canBeObjCSelectorComponent(*CurrentToken->Previous))
+CurrentToken->Previous->Type = TT_SelectorName;
   // determineStarAmpUsage() thinks that '*' '[' is allocating an
   // array of pointers, but if '[' starts a selector then '*' is a
   // binary operator.
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,10 @@
"  a = 42;\n"
"}];");
 
+  // Space between cast rparen and selector name component.
+  verifyFormat("[((Foo *)foo) bar];");
+  verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+
   // Message receiver taking multiple lines.
   Style.ColumnLimit = 20;
   // Non-corner case.


Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -501,6 +501,12 @@
 }
 if (StartsObjCMethodExpr && CurrentToken->Previous != Left) {
   CurrentToken->Type = TT_ObjCMethodExpr;
+  // If we haven't seen a colon yet, make sure the last identifier
+  // before the r_square is tagged as a selector name component.
+  if (!ColonFound && CurrentToken->Previous &&
+  CurrentToken->Previous->is(TT_Unknown) &&
+  canBeObjCSelectorComponent(*CurrentToken->Previous))
+CurrentToken->Previous->Type = TT_SelectorName;
   // determineStarAmpUsage() thinks that '*' '[' is allocating an
   // array of pointers, but if '[' starts a selector then '*' is a
   // binary operator.
Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,10 @@
"  a = 42;\n"
"}];");
 
+  // Space between cast rparen and selector name component.
+  verifyFormat("[((Foo *)foo) bar];");
+  verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+
   // Message receiver taking multiple lines.
   Style.ColumnLimit = 20;
   // Non-corner case.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47028: [clang-format/ObjC] Correctly annotate single-component ObjC method invocations

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: lib/Format/TokenAnnotator.cpp:510
+CurrentToken->Previous->Type = TT_SelectorName;
+  }
   // determineStarAmpUsage() thinks that '*' '[' is allocating an

jolesiak wrote:
> I'd remove braces for consistency.
Thanks, done.


Repository:
  rC Clang

https://reviews.llvm.org/D47028



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

[PATCH] D47028: [clang-format/ObjC] Correctly annotate single-component ObjC method invocations

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 147516.
benhamilton marked an inline comment as done.
benhamilton added a comment.

Style


Repository:
  rC Clang

https://reviews.llvm.org/D47028

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


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,10 @@
"  a = 42;\n"
"}];");
 
+  // Space between cast rparen and selector name component.
+  verifyFormat("[((Foo *)foo) bar];");
+  verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+
   // Message receiver taking multiple lines.
   Style.ColumnLimit = 20;
   // Non-corner case.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -501,6 +501,12 @@
 }
 if (StartsObjCMethodExpr && CurrentToken->Previous != Left) {
   CurrentToken->Type = TT_ObjCMethodExpr;
+  // If we haven't seen a colon yet, make sure the last identifier
+  // before the r_square is tagged as a selector name component.
+  if (!ColonFound && CurrentToken->Previous &&
+  CurrentToken->Previous->is(TT_Unknown) &&
+  canBeObjCSelectorComponent(*CurrentToken->Previous))
+CurrentToken->Previous->Type = TT_SelectorName;
   // determineStarAmpUsage() thinks that '*' '[' is allocating an
   // array of pointers, but if '[' starts a selector then '*' is a
   // binary operator.


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,10 @@
"  a = 42;\n"
"}];");
 
+  // Space between cast rparen and selector name component.
+  verifyFormat("[((Foo *)foo) bar];");
+  verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+
   // Message receiver taking multiple lines.
   Style.ColumnLimit = 20;
   // Non-corner case.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -501,6 +501,12 @@
 }
 if (StartsObjCMethodExpr && CurrentToken->Previous != Left) {
   CurrentToken->Type = TT_ObjCMethodExpr;
+  // If we haven't seen a colon yet, make sure the last identifier
+  // before the r_square is tagged as a selector name component.
+  if (!ColonFound && CurrentToken->Previous &&
+  CurrentToken->Previous->is(TT_Unknown) &&
+  canBeObjCSelectorComponent(*CurrentToken->Previous))
+CurrentToken->Previous->Type = TT_SelectorName;
   // determineStarAmpUsage() thinks that '*' '[' is allocating an
   // array of pointers, but if '[' starts a selector then '*' is a
   // binary operator.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D47028: [clang-format/ObjC] Correctly annotate single-component ObjC method invocations

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: djasper, jolesiak.
Herald added subscribers: cfe-commits, klimek.

Previously, clang-format's parser would fail to annotate the
selector in a single-component Objective-C method invocation with
`TT_SelectorName`. For example, the following:

  [foo bar];

would parse `bar` as `TT_Unknown`:

  M=0 C=1 T=Unknown S=0 B=0 BK=0 P=140 Name=identifier L=34 PPK=2
  FakeLParens= FakeRParens=0 II=0x559d5db51770 Text='bar'

This caused us to fail to insert a space after a closing cast rparen,
so the following:

  [((Foo *)foo) bar];

would format as:

  [((Foo *)foo)bar];

This diff fixes the issue by ensuring we annotate the selector
in a single-component Objective-C method invocation as
`TT_SelectorName`.

Test Plan: New tests added. Ran tests with:

  % make -j16 FormatTests && ./tools/clang/unittests/Format/FormatTests


Repository:
  rC Clang

https://reviews.llvm.org/D47028

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


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,10 @@
"  a = 42;\n"
"}];");
 
+  // Space between cast rparen and selector name component.
+  verifyFormat("[((Foo *)foo) bar];");
+  verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+
   // Message receiver taking multiple lines.
   Style.ColumnLimit = 20;
   // Non-corner case.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -501,6 +501,13 @@
 }
 if (StartsObjCMethodExpr && CurrentToken->Previous != Left) {
   CurrentToken->Type = TT_ObjCMethodExpr;
+  // If we haven't seen a colon yet, make sure the last identifier
+  // before the r_square is tagged as a selector name component.
+  if (!ColonFound && CurrentToken->Previous &&
+  CurrentToken->Previous->is(TT_Unknown) &&
+  canBeObjCSelectorComponent(*CurrentToken->Previous)) {
+CurrentToken->Previous->Type = TT_SelectorName;
+  }
   // determineStarAmpUsage() thinks that '*' '[' is allocating an
   // array of pointers, but if '[' starts a selector then '*' is a
   // binary operator.


Index: unittests/Format/FormatTestObjC.cpp
===
--- unittests/Format/FormatTestObjC.cpp
+++ unittests/Format/FormatTestObjC.cpp
@@ -792,6 +792,10 @@
"  a = 42;\n"
"}];");
 
+  // Space between cast rparen and selector name component.
+  verifyFormat("[((Foo *)foo) bar];");
+  verifyFormat("[((Foo *)foo) bar:1 blech:2];");
+
   // Message receiver taking multiple lines.
   Style.ColumnLimit = 20;
   // Non-corner case.
Index: lib/Format/TokenAnnotator.cpp
===
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -501,6 +501,13 @@
 }
 if (StartsObjCMethodExpr && CurrentToken->Previous != Left) {
   CurrentToken->Type = TT_ObjCMethodExpr;
+  // If we haven't seen a colon yet, make sure the last identifier
+  // before the r_square is tagged as a selector name component.
+  if (!ColonFound && CurrentToken->Previous &&
+  CurrentToken->Previous->is(TT_Unknown) &&
+  canBeObjCSelectorComponent(*CurrentToken->Previous)) {
+CurrentToken->Previous->Type = TT_SelectorName;
+  }
   // determineStarAmpUsage() thinks that '*' '[' is allocating an
   // array of pointers, but if '[' starts a selector then '*' is a
   // binary operator.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D46659: [clang-tidy/google-readability-casting] Disable check for Objective-C++

[Ben Hamilton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL332516: [clang-tidy/google-readability-casting] Disable 
check for Objective-C++ (authored by benhamilton, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D46659

Files:
  clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
  clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.mm

Index: clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -100,7 +100,8 @@
   }
 
   // The rest of this check is only relevant to C++.
-  if (!getLangOpts().CPlusPlus)
+  // We also disable it for Objective-C++.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC1 || getLangOpts().ObjC2)
 return;
   // Ignore code inside extern "C" {} blocks.
   if (!match(expr(hasAncestor(linkageSpecDecl())), *CastExpr, *Result.Context)
Index: clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.mm
===
--- clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.mm
+++ clang-tools-extra/trunk/test/clang-tidy/google-readability-casting.mm
@@ -0,0 +1,179 @@
+// RUN: clang-tidy %s -checks=-*,google-readability-casting -- \
+// RUN:   -xobjective-c++ -fobjc-abi-version=2 -fobjc-arc | count 0
+
+// Note: this test expects no diagnostics, but FileCheck cannot handle that,
+// hence the use of | count 0.
+
+bool g() { return false; }
+
+enum Enum { Enum1 };
+struct X {};
+struct Y : public X {};
+
+void f(int a, double b, const char *cpc, const void *cpv, X *pX) {
+
+  typedef const char *Typedef1;
+  typedef const char *Typedef2;
+  Typedef1 t1;
+  (Typedef2)t1;
+  (const char*)t1;
+  (Typedef1)cpc;
+
+  typedef char Char;
+  char *pc;
+  Char *pChar = (Char*)pc;
+
+  (Char)*cpc;
+
+  (char)*pChar;
+
+  (const char*)cpv;
+
+  char *pc2 = (char*)(cpc + 33);
+
+  const char &crc = *cpc;
+  char &rc = (char&)crc;
+
+  char &rc2 = (char&)*cpc;
+
+  char ** const* const* ppcpcpc;
+  char c = (char)ppcpcpc;
+
+  char ***pppc = (char***)*(ppcpcpc);
+
+  char ***pppc2 = (char***)(*ppcpcpc);
+
+  char *pc5 = (char*)(const char*)(cpv);
+
+  int b1 = (int)b;
+  b1 = (const int&)b;
+
+  b1 = (int) b;
+
+  b1 = (int) b;
+
+  b1 = (int) (b);
+
+  b1 = (int) (b);
+
+  Y *pB = (Y*)pX;
+  Y &rB = (Y&)*pX;
+
+  const char *pc3 = (const char*)cpv;
+
+  char *pc4 = (char*)cpv;
+
+  b1 = (int)Enum1;
+
+  Enum e = (Enum)b1;
+
+  int b2 = int(b);
+  int b3 = static_cast(b);
+  int b4 = b;
+  double aa = a;
+  (void)b2;
+  return (void)g();
+}
+
+template 
+void template_function(T t, int n) {
+  int i = (int)t;
+}
+
+template 
+struct TemplateStruct {
+  void f(T t, int n) {
+int k = (int)t;
+  }
+};
+
+void test_templates() {
+  template_function(1, 42);
+  template_function(1.0, 42);
+  TemplateStruct().f(1, 42);
+  TemplateStruct().f(1.0, 42);
+}
+
+extern "C" {
+void extern_c_code(const char *cpc) {
+  char *pc = (char*)cpc;
+}
+}
+
+#define CAST(type, value) (type)(value)
+void macros(double d) {
+  int i = CAST(int, d);
+}
+
+enum E { E1 = 1 };
+template 
+struct A {
+  // Usage of template argument e = E1 is represented as (E)1 in the AST for
+  // some reason. We have a special treatment of this case to avoid warnings
+  // here.
+  static const E ee = e;
+};
+struct B : public A {};
+
+
+void overloaded_function();
+void overloaded_function(int);
+
+template
+void g(Fn fn) {
+  fn();
+}
+
+void function_casts() {
+  typedef void (*FnPtrVoid)();
+  typedef void (&FnRefVoid)();
+  typedef void (&FnRefInt)(int);
+
+  g((void (*)())overloaded_function);
+  g((void (*)())&overloaded_function);
+  g((void (&)())overloaded_function);
+
+  g((FnPtrVoid)overloaded_function);
+  g((FnPtrVoid)&overloaded_function);
+  g((FnRefVoid)overloaded_function);
+
+  FnPtrVoid fn0 = (void (*)())&overloaded_function;
+  FnPtrVoid fn1 = (void (*)())overloaded_function;
+  FnPtrVoid fn1a = (FnPtrVoid)overloaded_function;
+  FnRefInt fn2 = (void (&)(int))overloaded_function;
+  auto fn3 = (void (*)())&overloaded_function;
+  auto fn4 = (void (*)())overloaded_function;
+  auto fn5 = (void (&)(int))overloaded_function;
+
+  void (*fn6)() = (void (*)())&overloaded_function;
+  void (*fn7)() = (void (*)())overloaded_function;
+  void (*fn8)() = (FnPtrVoid)overloaded_function;
+  void (&fn9)(int) = (void (&)(int))overloaded_function;
+
+  void (*correct1)() = static_cast(overloaded_function);
+  FnPtrVoid correct2 = static_cast(&overloaded_function);
+  FnRefInt correct3 = static_cast(overloaded_function);
+}
+
+struct S {
+S(const char *);
+};
+struct ConvertibleToS {
+  operator S() const;
+};
+struct ConvertibleToSRef {
+  operator const S&() const;
+};
+
+

[PATCH] D46659: [clang-tidy/google-readability-casting] Disable check for Objective-C++

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added inline comments.



Comment at: clang-tidy/google/AvoidCStyleCastsCheck.cpp:103-104
   // The rest of this check is only relevant to C++.
-  if (!getLangOpts().CPlusPlus)
+  // We also disable it for Objective-C++.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC1 || getLangOpts().ObjC2)
 return;

stephanemoore wrote:
> In the future it would probably be good to allow configuring whether or not 
> this is disabled but I think that disabling it for Objective-C++ completely 
> in the interim is a positive change.
Agreed. Filed https://bugs.llvm.org/show_bug.cgi?id=37489 to follow up.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46659



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

[PATCH] D46659: [clang-tidy/google-readability-casting] Disable check for Objective-C++

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

After discussion, I changed this diff to simply disable the check altogether 
for Objective-C++.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46659



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

[PATCH] D46659: [clang-tidy/google-readability-casting] Allow C-style casts to/from Objective-C object types

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton updated this revision to Diff 147102.
benhamilton added a comment.

- Change to simply disable check for Objective-C++


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46659

Files:
  clang-tidy/google/AvoidCStyleCastsCheck.cpp
  test/clang-tidy/google-readability-casting.mm

Index: test/clang-tidy/google-readability-casting.mm
===
--- /dev/null
+++ test/clang-tidy/google-readability-casting.mm
@@ -0,0 +1,179 @@
+// RUN: clang-tidy %s -checks=-*,google-readability-casting -- \
+// RUN:   -xobjective-c++ -fobjc-abi-version=2 -fobjc-arc | count 0
+
+// Note: this test expects no diagnostics, but FileCheck cannot handle that,
+// hence the use of | count 0.
+
+bool g() { return false; }
+
+enum Enum { Enum1 };
+struct X {};
+struct Y : public X {};
+
+void f(int a, double b, const char *cpc, const void *cpv, X *pX) {
+
+  typedef const char *Typedef1;
+  typedef const char *Typedef2;
+  Typedef1 t1;
+  (Typedef2)t1;
+  (const char*)t1;
+  (Typedef1)cpc;
+
+  typedef char Char;
+  char *pc;
+  Char *pChar = (Char*)pc;
+
+  (Char)*cpc;
+
+  (char)*pChar;
+
+  (const char*)cpv;
+
+  char *pc2 = (char*)(cpc + 33);
+
+  const char &crc = *cpc;
+  char &rc = (char&)crc;
+
+  char &rc2 = (char&)*cpc;
+
+  char ** const* const* ppcpcpc;
+  char c = (char)ppcpcpc;
+
+  char ***pppc = (char***)*(ppcpcpc);
+
+  char ***pppc2 = (char***)(*ppcpcpc);
+
+  char *pc5 = (char*)(const char*)(cpv);
+
+  int b1 = (int)b;
+  b1 = (const int&)b;
+
+  b1 = (int) b;
+
+  b1 = (int) b;
+
+  b1 = (int) (b);
+
+  b1 = (int) (b);
+
+  Y *pB = (Y*)pX;
+  Y &rB = (Y&)*pX;
+
+  const char *pc3 = (const char*)cpv;
+
+  char *pc4 = (char*)cpv;
+
+  b1 = (int)Enum1;
+
+  Enum e = (Enum)b1;
+
+  int b2 = int(b);
+  int b3 = static_cast(b);
+  int b4 = b;
+  double aa = a;
+  (void)b2;
+  return (void)g();
+}
+
+template 
+void template_function(T t, int n) {
+  int i = (int)t;
+}
+
+template 
+struct TemplateStruct {
+  void f(T t, int n) {
+int k = (int)t;
+  }
+};
+
+void test_templates() {
+  template_function(1, 42);
+  template_function(1.0, 42);
+  TemplateStruct().f(1, 42);
+  TemplateStruct().f(1.0, 42);
+}
+
+extern "C" {
+void extern_c_code(const char *cpc) {
+  char *pc = (char*)cpc;
+}
+}
+
+#define CAST(type, value) (type)(value)
+void macros(double d) {
+  int i = CAST(int, d);
+}
+
+enum E { E1 = 1 };
+template 
+struct A {
+  // Usage of template argument e = E1 is represented as (E)1 in the AST for
+  // some reason. We have a special treatment of this case to avoid warnings
+  // here.
+  static const E ee = e;
+};
+struct B : public A {};
+
+
+void overloaded_function();
+void overloaded_function(int);
+
+template
+void g(Fn fn) {
+  fn();
+}
+
+void function_casts() {
+  typedef void (*FnPtrVoid)();
+  typedef void (&FnRefVoid)();
+  typedef void (&FnRefInt)(int);
+
+  g((void (*)())overloaded_function);
+  g((void (*)())&overloaded_function);
+  g((void (&)())overloaded_function);
+
+  g((FnPtrVoid)overloaded_function);
+  g((FnPtrVoid)&overloaded_function);
+  g((FnRefVoid)overloaded_function);
+
+  FnPtrVoid fn0 = (void (*)())&overloaded_function;
+  FnPtrVoid fn1 = (void (*)())overloaded_function;
+  FnPtrVoid fn1a = (FnPtrVoid)overloaded_function;
+  FnRefInt fn2 = (void (&)(int))overloaded_function;
+  auto fn3 = (void (*)())&overloaded_function;
+  auto fn4 = (void (*)())overloaded_function;
+  auto fn5 = (void (&)(int))overloaded_function;
+
+  void (*fn6)() = (void (*)())&overloaded_function;
+  void (*fn7)() = (void (*)())overloaded_function;
+  void (*fn8)() = (FnPtrVoid)overloaded_function;
+  void (&fn9)(int) = (void (&)(int))overloaded_function;
+
+  void (*correct1)() = static_cast(overloaded_function);
+  FnPtrVoid correct2 = static_cast(&overloaded_function);
+  FnRefInt correct3 = static_cast(overloaded_function);
+}
+
+struct S {
+S(const char *);
+};
+struct ConvertibleToS {
+  operator S() const;
+};
+struct ConvertibleToSRef {
+  operator const S&() const;
+};
+
+void conversions() {
+  //auto s1 = (const S&)"";
+  auto s2 = (S)"";
+  auto s2a = (struct S)"";
+  auto s2b = (const S)"";
+  ConvertibleToS c;
+  auto s3 = (const S&)c;
+  auto s4 = (S)c;
+  ConvertibleToSRef cr;
+  auto s5 = (const S&)cr;
+  auto s6 = (S)cr;
+}
Index: clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -100,7 +100,8 @@
   }
 
   // The rest of this check is only relevant to C++.
-  if (!getLangOpts().CPlusPlus)
+  // We also disable it for Objective-C++.
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC1 || getLangOpts().ObjC2)
 return;
   // Ignore code inside extern "C" {} blocks.
   if (!match(expr(hasAncestor(linkageSpecDecl())), *CastExpr, *Result.Context)
___
cfe-commits mailing list

[PATCH] D46659: [clang-tidy/google-readability-casting] Allow C-style casts to/from Objective-C object types

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton added a comment.

An alternative implementation would be to allow C-style casts (either always or 
only for ObjC objects) within Objective-C methods inside Objective-C++ files, 
but that may get messy with things like shared macros.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46659



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

[PATCH] D46659: [clang-tidy/google-readability-casting] Allow C-style casts to/from Objective-C object types

[Ben Hamilton via Phabricator via cfe-commits]

benhamilton created this revision.
benhamilton added reviewers: alexfh, Wizard, hokein.
Herald added a subscriber: cfe-commits.

Previously, `google-readability-casting` would trigger for
Objective-C++ code using C-style casts to or from Objective-C object
types.

The official Google Objective-C standard says Objective-C++ allows
authors to mix the Objective-C style (which uses C-style casts) and
C++ (which disallows it):

http://google.github.io/styleguide/objcguide.html#style-matches-the-language

So, to resolve this conflict, this diff updates
`google-readability-casting` to ignore C-style casts to or from
Objective-C object types.

Test Plan: New tests added. Ran tests with:

  % make -j16 check-clang-tools
  Before diff, confirmed tests failed:
  https://reviews.llvm.org/P8081
  After diff, confirrmed tests passed.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46659

Files:
  clang-tidy/google/AvoidCStyleCastsCheck.cpp
  test/clang-tidy/google-readability-casting.mm


Index: test/clang-tidy/google-readability-casting.mm
===
--- /dev/null
+++ test/clang-tidy/google-readability-casting.mm
@@ -0,0 +1,42 @@
+// RUN: clang-tidy %s -checks=-*,google-readability-casting -- \
+// RUN:   -xobjective-c++ -fobjc-abi-version=2 -fobjc-arc | count 0
+
+// Note: this test expects no diagnostics, but FileCheck cannot handle that,
+// hence the use of | count 0.
+
+#define nil 0
+
+@interface Foo
+@end
+
+@protocol Proto
+@end
+
+@interface Bar : Foo 
+@end
+
+@interface Baz : Foo 
+@end
+
+void foo() {
+  id nilObj = nil;
+  Foo *foo = (Foo *)nilObj;
+  Bar *bar = (Bar *)nilObj;
+  id ego = (id)foo;
+  foo = (Foo *)bar;
+  foo = (id)bar;
+  id nilProto = (id)bar;
+  ego = (id)nilProto;
+  bar = (Bar *)nilProto;
+  Foo *fooProto = (Foo *)bar;
+  Baz *baz = (Baz *)bar;
+  Class klass = (Class)nilObj;
+  ego = (id)klass;
+  void *voidStar = nullptr;
+  foo = (__bridge Foo *)voidStar;
+  nilProto = (__bridge id)voidStar;
+  klass = (__bridge Class)voidStar;
+  voidStar = (__bridge void *)foo;
+  voidStar = (__bridge void *)nilProto;
+  voidStar = (__bridge void *)klass;
+}
Index: clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -110,6 +110,10 @@
   // compiled as C++.
   if (getCurrentMainFile().endswith(".c"))
 return;
+  // Ignore casts between Objective-C types.
+  if (SourceType->isObjCObjectPointerType() ||
+  DestType->isObjCObjectPointerType())
+return;
 
   SourceManager &SM = *Result.SourceManager;
 


Index: test/clang-tidy/google-readability-casting.mm
===
--- /dev/null
+++ test/clang-tidy/google-readability-casting.mm
@@ -0,0 +1,42 @@
+// RUN: clang-tidy %s -checks=-*,google-readability-casting -- \
+// RUN:   -xobjective-c++ -fobjc-abi-version=2 -fobjc-arc | count 0
+
+// Note: this test expects no diagnostics, but FileCheck cannot handle that,
+// hence the use of | count 0.
+
+#define nil 0
+
+@interface Foo
+@end
+
+@protocol Proto
+@end
+
+@interface Bar : Foo 
+@end
+
+@interface Baz : Foo 
+@end
+
+void foo() {
+  id nilObj = nil;
+  Foo *foo = (Foo *)nilObj;
+  Bar *bar = (Bar *)nilObj;
+  id ego = (id)foo;
+  foo = (Foo *)bar;
+  foo = (id)bar;
+  id nilProto = (id)bar;
+  ego = (id)nilProto;
+  bar = (Bar *)nilProto;
+  Foo *fooProto = (Foo *)bar;
+  Baz *baz = (Baz *)bar;
+  Class klass = (Class)nilObj;
+  ego = (id)klass;
+  void *voidStar = nullptr;
+  foo = (__bridge Foo *)voidStar;
+  nilProto = (__bridge id)voidStar;
+  klass = (__bridge Class)voidStar;
+  voidStar = (__bridge void *)foo;
+  voidStar = (__bridge void *)nilProto;
+  voidStar = (__bridge void *)klass;
+}
Index: clang-tidy/google/AvoidCStyleCastsCheck.cpp
===
--- clang-tidy/google/AvoidCStyleCastsCheck.cpp
+++ clang-tidy/google/AvoidCStyleCastsCheck.cpp
@@ -110,6 +110,10 @@
   // compiled as C++.
   if (getCurrentMainFile().endswith(".c"))
 return;
+  // Ignore casts between Objective-C types.
+  if (SourceType->isObjCObjectPointerType() ||
+  DestType->isObjCObjectPointerType())
+return;
 
   SourceManager &SM = *Result.SourceManager;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits