[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜
stephanemoore updated this revision to Diff 164568.
stephanemoore marked 3 inline comments as done.
stephanemoore edited the summary of this revision.
stephanemoore added a comment.
Implemented the following suggested changes:
• Restricted matching to function declarations that are not in expansions in
system headers.
• Extended the check to all function declarations rather than just function
definitions.
• Implemented a fixit hint for functions of static storage class.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51575
Files:
clang-tidy/google/CMakeLists.txt
clang-tidy/google/FunctionNamingCheck.cpp
clang-tidy/google/FunctionNamingCheck.h
clang-tidy/google/GoogleTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/google-objc-function-naming.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/google-objc-function-naming.m
Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+ google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements)
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+ static bool is_positive(int i) { return i > 0; }
+ bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corre
[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜
stephanemoore updated this revision to Diff 164569.
stephanemoore added a comment.
Implemented the following suggested changes:
• Added a note that FunctionNamingCheck does not apply to Objective-C method
name or property declarations.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51575
Files:
clang-tidy/google/CMakeLists.txt
clang-tidy/google/FunctionNamingCheck.cpp
clang-tidy/google/FunctionNamingCheck.h
clang-tidy/google/GoogleTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/google-objc-function-naming.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/google-objc-function-naming.m
Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+ google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements)
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+ static bool is_positive(int i) { return i > 0; }
+ bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+ static bool IsPositive(int i) { return i > 0; }
+ bool *ABCIsNegative(int i) { return i < 0; }
Index: docs/ReleaseNotes.rst
==
[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜
stephanemoore updated this revision to Diff 164570.
stephanemoore marked an inline comment as done.
stephanemoore added a comment.
Cleaned up comment about FunctionNamingCheck not applying to Objective-C method
or property declarations.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51575
Files:
clang-tidy/google/CMakeLists.txt
clang-tidy/google/FunctionNamingCheck.cpp
clang-tidy/google/FunctionNamingCheck.h
clang-tidy/google/GoogleTidyModule.cpp
docs/ReleaseNotes.rst
docs/clang-tidy/checks/google-objc-function-naming.rst
docs/clang-tidy/checks/list.rst
test/clang-tidy/google-objc-function-naming.m
Index: test/clang-tidy/google-objc-function-naming.m
===
--- /dev/null
+++ test/clang-tidy/google-objc-function-naming.m
@@ -0,0 +1,49 @@
+// RUN: %check_clang_tidy %s google-objc-function-naming %t
+
+typedef _Bool bool;
+
+static bool ispositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'ispositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool Ispositive(int a) { return a > 0; }
+
+static bool is_positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'is_positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool isPositive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'isPositive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool Is_Positive(int a) { return a > 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: function name 'Is_Positive' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static bool IsPositive(int a) { return a > 0; }
+
+static bool IsPositive(int a) { return a > 0; }
+
+bool ispalindrome(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ispalindrome' not using function naming conventions described by Google Objective-C style guide
+
+static const char *md5(const char *str) { return 0; }
+// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: function name 'md5' not using function naming conventions described by Google Objective-C style guide
+// CHECK-FIXES: static const char *Md5(const char *str) { return 0; }
+
+static const char *MD5(const char *str) { return 0; }
+
+static const char *URL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *DEFFooURL(void) { return "https://clang.llvm.org/";; }
+
+static const char *StringFromNSString(id str) { return ""; }
+
+void ABLog_String(const char *str);
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: function name 'ABLog_String' not using function naming conventions described by Google Objective-C style guide
+
+void ABLogString(const char *str);
+
+const char *ABURL(void) { return "https://clang.llvm.org/";; }
+
+const char *ABFooURL(void) { return "https://clang.llvm.org/";; }
+
+int main(int argc, const char **argv) { return 0; }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -118,6 +118,7 @@
google-explicit-constructor
google-global-names-in-headers
google-objc-avoid-throwing-exception
+ google-objc-function-naming
google-objc-global-variable-declaration
google-readability-braces-around-statements (redirects to readability-braces-around-statements)
google-readability-casting
Index: docs/clang-tidy/checks/google-objc-function-naming.rst
===
--- /dev/null
+++ docs/clang-tidy/checks/google-objc-function-naming.rst
@@ -0,0 +1,27 @@
+.. title:: clang-tidy - google-objc-function-naming
+
+google-objc-function-naming
+===
+
+Finds function declarations in Objective-C files that do not follow the pattern
+described in the Google Objective-C Style Guide.
+
+The corresponding style guide rule can be found here:
+https://google.github.io/styleguide/objcguide.html#function-names
+
+All function names should be in upper camel case. Functions whose storage class
+is not static should have an appropriate prefix.
+
+The following code sample does not follow this pattern:
+
+.. code-block:: objc
+
+ static bool is_positive(int i) { return i > 0; }
+ bool IsNegative(int i) { return i < 0; }
+
+The sample above might be corrected to the following code:
+
+.. code-block:: objc
+
+ static bool IsPositive(int i) { return i > 0; }
+ bool *ABCIsNegative(int i) { return i < 0; }
Index: docs/ReleaseNotes.rst
=
[PATCH] D51575: [clang-tidy] Implement a clang-tidy check to verify Google Objective-C function naming conventions 📜
stephanemoore added inline comments.
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:57
+ functionDecl(
+ isDefinition(),
+ unless(anyOf(isMain(), matchesName(validFunctionNameRegex(true)),
hokein wrote:
> stephanemoore wrote:
> > hokein wrote:
> > > any reason why we restrict to definitions only? I think we can consider
> > > declarations too.
> > I restricted the check to function definitions because function
> > declarations are sometimes associated with functions outside the control of
> > the author. I have personally observed unfortunate cases where functions
> > declared in the iOS SDK had incorrect—or seemingly incorrect—availability
> > attributes that caused fatal dyld assertions during application startup.
> > The check currently intends to avoid flagging function declarations because
> > of the rare circumstances where an inflexible function declaration without
> > a corresponding function definition is required.
> >
> > I have added a comment explaining why only function definitions are flagged.
> >
> > I am still open to discussion though.
> Thanks for the explanations.
>
> I have a concern about the heuristic using here, it seems fragile -- if there
> is an inline function defined in a base header, the check will still give a
> warning to it if the source file `.m` #includes this header; it also limits
> the scope of the check, I think this check is flagged mostly on file-local
> functions (e.g. static functions, functions defined in anonymous namespace).
>
> Flagging on function declaration doesn't seem too problematic (we already
> have a similar check `objc-property-declaration` does the same thing) -- our
> internal review system shows check warnings on changed code, if user's code
> includes a common header which violates the check, warnings on the header
> will not be shown; for violations in iOS SDK, we can use some filtering
> matchers (`isExpansionInSystemHeader` maybe work) to ignore all functions
> from these files.
>
>
Good idea to use isExpansionInSystemHeader. I wasn't aware of that particular
matcher. I have incorporated that into the matching.
I am still wary about flagging function declarations but I think that false
positives should generally be marginal and we can monitor for them. I have
extended the check to also include function declarations.
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:
> 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.)
>
I have been meaning to send out a proposal to change the
`objc-property-declaration` check to allow arbitrary capitalization but I ended
up sending this out first. Let me prep my proposal for that change
simultaneously.
Comment at: clang-tidy/google/FunctionNamingCheck.cpp:50
+
+void FunctionNamingCheck::registerMatchers(MatchFinder *Finder) {
+ // This check should only be applied to Objective-C sources.
benhamilton wrote:
> 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.
Implemented a fixit hint for functions of static storage class.
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
benhamilton wrote:
> Worth mentioning this does not apply to Objective-C method names, nor
> Objective-C properties.
>
Added a note that this check does not apply to Objective-C methods or
properties.
[PATCH] D43630: [Driver] Fix search paths on x32
sylvestre.ledru added a comment. @jrtc27 are you going to take care of the tests? Thanks Repository: rC Clang https://reviews.llvm.org/D43630 ___ 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 🔧
stephanemoore created this revision.
stephanemoore added reviewers: benhamilton, Wizard.
Herald added subscribers: cfe-commits, jfb.
§1 Description
This changes the objc-property-declaration check to allow arbitrary acronyms
and initialisms instead of using whitelisted acronyms. In Objective-C it is
relatively common to use project prefixes in property names for the purposes of
disambiguation. For example, the CIColor¹ and CGColor² properties on UIColor
both represent symbol prefixes being used in proeprty names outside of Apple's
accepted acronyms³. The union of Apple's accepted acronyms and all symbol
prefixes that might be used for disambiguation in property declarations
effectively allows for any arbitrary sequence of capital alphanumeric
characters to be acceptable in property declarations. This change updates the
check accordingly.
The test variants with custom configurations are deleted as part of this change
because their configurations no longer impact behavior. The acronym
configurations are currently preserved for backwards compatibility of check
configuration.
[1]
https://developer.apple.com/documentation/uikit/uicolor/1621951-cicolor?language=objc
[2]
https://developer.apple.com/documentation/uikit/uicolor/1621954-cgcolor?language=objc
[3]
https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/CodingGuidelines/Articles/APIAbbreviations.html#//apple_ref/doc/uid/20001285-BCIHCGAE
§2 Test Notes
Changes verified by:
• Running clang-tidy unit tests.
• Used check_clang_tidy.py to verify expected output of processing
objc-property-declaration.m
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51832
Files:
clang-tidy/objc/PropertyDeclarationCheck.cpp
test/clang-tidy/objc-property-declaration-additional.m
test/clang-tidy/objc-property-declaration-custom.m
test/clang-tidy/objc-property-declaration.m
Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -2,6 +2,9 @@
@class NSData;
@class NSString;
@class UIViewController;
+@class CIColor;
+
+typedef void *CGColorRef;
@interface Foo
@property(assign, nonatomic) int NotCamelCase;
@@ -23,6 +26,9 @@
@property(assign, nonatomic) int enableGLAcceleration;
@property(assign, nonatomic) int ID;
@property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
@end
@interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN: [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}, \
-// RUN: {key: objc-property-declaration.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN: [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_cust
r341754 - Distinguish `__block` variables that are captured by escaping blocks
Author: ahatanak
Date: Sat Sep 8 13:03:00 2018
New Revision: 341754
URL: http://llvm.org/viewvc/llvm-project?rev=341754&view=rev
Log:
Distinguish `__block` variables that are captured by escaping blocks
from those that aren't.
This patch changes the way __block variables that aren't captured by
escaping blocks are handled:
- Since non-escaping blocks on the stack never get copied to the heap
(see https://reviews.llvm.org/D49303), Sema shouldn't error out when
the type of a non-escaping __block variable doesn't have an accessible
copy constructor.
- IRGen doesn't have to use the specialized byref structure (see
https://clang.llvm.org/docs/Block-ABI-Apple.html#id8) for a
non-escaping __block variable anymore. Instead IRGen can emit the
variable as a normal variable and copy the reference to the block
literal. Byref copy/dispose helpers aren't needed either.
rdar://problem/39352313
Differential Revision: https://reviews.llvm.org/D51564
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/Sema/ScopeInfo.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/ScopeInfo.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/CodeGen/block-byref-aggr.c
cfe/trunk/test/CodeGen/blocks-seq.c
cfe/trunk/test/CodeGen/exceptions.c
cfe/trunk/test/CodeGen/personality.c
cfe/trunk/test/CodeGenCXX/block-capture.cpp
cfe/trunk/test/CodeGenCXX/blocks.cpp
cfe/trunk/test/CodeGenCXX/debug-info-blocks.cpp
cfe/trunk/test/CodeGenCXX/noescape.cpp
cfe/trunk/test/CodeGenObjC/arc-no-arc-exceptions.m
cfe/trunk/test/CodeGenObjC/arc-unoptimized-byref-var.m
cfe/trunk/test/CodeGenObjC/blocks-1.m
cfe/trunk/test/CodeGenObjC/noescape.m
cfe/trunk/test/CodeGenObjCXX/arc-blocks.mm
cfe/trunk/test/PCH/block-helpers.cpp
cfe/trunk/test/SemaObjCXX/blocks.mm
cfe/trunk/test/SemaObjCXX/noescape.mm
Modified: cfe/trunk/include/clang/AST/Decl.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=341754&r1=341753&r2=341754&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Sat Sep 8 13:03:00 2018
@@ -965,6 +965,8 @@ protected:
/// Defines kind of the ImplicitParamDecl: 'this', 'self', 'vtt', '_cmd' or
/// something else.
unsigned ImplicitParamKind : 3;
+
+unsigned EscapingByref : 1;
};
union {
@@ -1407,6 +1409,19 @@ public:
NonParmVarDeclBits.PreviousDeclInSameBlockScope = Same;
}
+ /// Indicates the capture is a __block variable that is captured by a block
+ /// that can potentially escape (a block for which BlockDecl::doesNotEscape
+ /// returns false).
+ bool isEscapingByref() const;
+
+ /// Indicates the capture is a __block variable that is never captured by an
+ /// escaping block.
+ bool isNonEscapingByref() const;
+
+ void setEscapingByref() {
+NonParmVarDeclBits.EscapingByref = true;
+ }
+
/// Retrieve the variable declaration from which this variable could
/// be instantiated, if it is an instantiation (rather than a non-template).
VarDecl *getTemplateInstantiationPattern() const;
@@ -3858,6 +3873,14 @@ public:
/// variable.
bool isByRef() const { return VariableAndFlags.getInt() & flag_isByRef; }
+bool isEscapingByref() const {
+ return getVariable()->isEscapingByref();
+}
+
+bool isNonEscapingByref() const {
+ return getVariable()->isNonEscapingByref();
+}
+
/// Whether this is a nested capture, i.e. the variable captured
/// is not from outside the immediately enclosing function/block.
bool isNested() const { return VariableAndFlags.getInt() & flag_isNested; }
Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=341754&r1=341753&r2=341754&view=diff
==
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Sat Sep 8 13:03:00 2018
@@ -31,6 +31,7 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
+#include "llvm/ADT/TinyPtrVector.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/ErrorHandling.h"
#include
@@ -202,6 +203,12 @@ public:
/// function.
SmallVector CompoundScopes;
+ /// The set of blocks that are introduced in this function.
+ llvm::SmallPtrSet Blocks;
+
+ /// The set of __block variables that are introduced in this function.
+
[PATCH] D51564: Distinguish `__block` variables that are captured by escaping blocks from those that aren't
This revision was automatically updated to reflect the committed changes.
Closed by commit rL341754: Distinguish `__block` variables that are captured by
escaping blocks (authored by ahatanak, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
https://reviews.llvm.org/D51564?vs=164069&id=164576#toc
Repository:
rL LLVM
https://reviews.llvm.org/D51564
Files:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/Sema/ScopeInfo.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/ScopeInfo.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/CodeGen/block-byref-aggr.c
cfe/trunk/test/CodeGen/blocks-seq.c
cfe/trunk/test/CodeGen/exceptions.c
cfe/trunk/test/CodeGen/personality.c
cfe/trunk/test/CodeGenCXX/block-capture.cpp
cfe/trunk/test/CodeGenCXX/blocks.cpp
cfe/trunk/test/CodeGenCXX/debug-info-blocks.cpp
cfe/trunk/test/CodeGenCXX/noescape.cpp
cfe/trunk/test/CodeGenObjC/arc-no-arc-exceptions.m
cfe/trunk/test/CodeGenObjC/arc-unoptimized-byref-var.m
cfe/trunk/test/CodeGenObjC/blocks-1.m
cfe/trunk/test/CodeGenObjC/noescape.m
cfe/trunk/test/CodeGenObjCXX/arc-blocks.mm
cfe/trunk/test/PCH/block-helpers.cpp
cfe/trunk/test/SemaObjCXX/blocks.mm
cfe/trunk/test/SemaObjCXX/noescape.mm
Index: cfe/trunk/lib/CodeGen/CGDecl.cpp
===
--- cfe/trunk/lib/CodeGen/CGDecl.cpp
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp
@@ -1212,8 +1212,8 @@
AutoVarEmission emission(D);
- bool isByRef = D.hasAttr();
- emission.IsByRef = isByRef;
+ bool isEscapingByRef = D.isEscapingByref();
+ emission.IsEscapingByRef = isEscapingByRef;
CharUnits alignment = getContext().getDeclAlign(&D);
@@ -1252,8 +1252,8 @@
// in OpenCL.
if ((!getLangOpts().OpenCL ||
Ty.getAddressSpace() == LangAS::opencl_constant) &&
- (CGM.getCodeGenOpts().MergeAllConstants && !NRVO && !isByRef &&
- CGM.isTypeConstant(Ty, true))) {
+ (CGM.getCodeGenOpts().MergeAllConstants && !NRVO &&
+ !isEscapingByRef && CGM.isTypeConstant(Ty, true))) {
EmitStaticVarDecl(D, llvm::GlobalValue::InternalLinkage);
// Signal this condition to later callbacks.
@@ -1305,7 +1305,7 @@
} else {
CharUnits allocaAlignment;
llvm::Type *allocaTy;
- if (isByRef) {
+ if (isEscapingByRef) {
auto &byrefInfo = getBlockByrefInfo(&D);
allocaTy = byrefInfo.Type;
allocaAlignment = byrefInfo.ByrefAlignment;
@@ -1505,7 +1505,7 @@
}
// Initialize the structure of a __block variable.
- if (emission.IsByRef)
+ if (emission.IsEscapingByRef)
emitByrefStructureInit(emission);
// Initialize the variable here if it doesn't have a initializer and it is a
@@ -1515,7 +1515,7 @@
type.isNonTrivialToPrimitiveDefaultInitialize() ==
QualType::PDIK_Struct) {
LValue Dst = MakeAddrLValue(emission.getAllocatedAddress(), type);
-if (emission.IsByRef)
+if (emission.IsEscapingByRef)
drillIntoBlockVariable(*this, Dst, &D);
defaultInitNonTrivialCStructVar(Dst);
return;
@@ -1527,7 +1527,7 @@
// Check whether this is a byref variable that's potentially
// captured and moved by its own initializer. If so, we'll need to
// emit the initializer first, then copy into the variable.
- bool capturedByInit = emission.IsByRef && isCapturedBy(D, Init);
+ bool capturedByInit = emission.IsEscapingByRef && isCapturedBy(D, Init);
Address Loc =
capturedByInit ? emission.Addr : emission.getObjectAddress(*this);
@@ -1721,7 +1721,8 @@
// If this is a block variable, call _Block_object_destroy
// (on the unforwarded address). Don't enter this cleanup if we're in pure-GC
// mode.
- if (emission.IsByRef && CGM.getLangOpts().getGC() != LangOptions::GCOnly) {
+ if (emission.IsEscapingByRef &&
+ CGM.getLangOpts().getGC() != LangOptions::GCOnly) {
BlockFieldFlags Flags = BLOCK_FIELD_IS_BYREF;
if (emission.Variable->getType().isObjCGCWeak())
Flags |= BLOCK_FIELD_IS_WEAK;
Index: cfe/trunk/lib/CodeGen/CGExpr.cpp
===
--- cfe/trunk/lib/CodeGen/CGExpr.cpp
+++ cfe/trunk/lib/CodeGen/CGExpr.cpp
@@ -2486,7 +2486,7 @@
}
assert(isa(CurCodeDecl));
- Address addr = GetAddrOfBlockDecl(VD, VD->hasAttr());
+ Address addr = GetAddrOfBlockDecl(VD);
return MakeAddrLValue(addr, T, AlignmentSource::Decl);
}
}
@@ -2538,7 +2538,7 @@
}
// Drill into block byref variables.
-bool isBlockByref = VD->hasAttr();
+bool isBlockByref =
r341755 - ms: Insert $$Z in mangling between directly consecutive parameter packs.
Author: nico
Date: Sat Sep 8 13:58:39 2018
New Revision: 341755
URL: http://llvm.org/viewvc/llvm-project?rev=341755&view=rev
Log:
ms: Insert $$Z in mangling between directly consecutive parameter packs.
Fixes PR38783.
Differential Revision: https://reviews.llvm.org/D51784
Modified:
cfe/trunk/lib/AST/MicrosoftMangle.cpp
cfe/trunk/test/CodeGenCXX/mangle-ms-templates.cpp
Modified: cfe/trunk/lib/AST/MicrosoftMangle.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/MicrosoftMangle.cpp?rev=341755&r1=341754&r2=341755&view=diff
==
--- cfe/trunk/lib/AST/MicrosoftMangle.cpp (original)
+++ cfe/trunk/lib/AST/MicrosoftMangle.cpp Sat Sep 8 13:58:39 2018
@@ -1384,9 +1384,16 @@ void MicrosoftCXXNameMangler::mangleTemp
assert(TPL->size() == TemplateArgs.size() &&
"size mismatch between args and parms!");
- unsigned Idx = 0;
- for (const TemplateArgument &TA : TemplateArgs.asArray())
-mangleTemplateArg(TD, TA, TPL->getParam(Idx++));
+ for (size_t i = 0; i < TemplateArgs.size(); ++i) {
+const TemplateArgument &TA = TemplateArgs[i];
+
+// Separate consecutive packs by $$Z.
+if (i > 0 && TA.getKind() == TemplateArgument::Pack &&
+TemplateArgs[i - 1].getKind() == TemplateArgument::Pack)
+ Out << "$$Z";
+
+mangleTemplateArg(TD, TA, TPL->getParam(i));
+ }
}
void MicrosoftCXXNameMangler::mangleTemplateArg(const TemplateDecl *TD,
Modified: cfe/trunk/test/CodeGenCXX/mangle-ms-templates.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/mangle-ms-templates.cpp?rev=341755&r1=341754&r2=341755&view=diff
==
--- cfe/trunk/test/CodeGenCXX/mangle-ms-templates.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/mangle-ms-templates.cpp Sat Sep 8 13:58:39 2018
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -fms-extensions
-fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -fms-extensions
-fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck -check-prefix
X64 %s
+// RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o -
-fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o -
-fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck
-check-prefix X64 %s
template
class Class {
@@ -185,13 +185,35 @@ void spam() {
// Unlike Itanium, there is no character code to indicate an argument pack.
// Tested with MSVC 2013, the first version which supports variadic templates.
-template void variadic_fn_template(const Ts &...args) { }
+template void variadic_fn_template(const Ts &...args);
+template
+void multi_variadic_fn(Ts... ts, Us... us);
+template
+void multi_variadic_mixed(Ts... ts, C c, Us... us);
void variadic_fn_instantiate() {
variadic_fn_template(0, 1, 3, 4);
variadic_fn_template(0, 1, 'a', "b");
+
+ // Directlly consecutive packs are separated by $$Z...
+ multi_variadic_fn(1, 2, 3, 4, 5);
+ multi_variadic_fn(1, 2, 3, 4, 5);
+
+ // ...but not if another template parameter is between them.
+ multi_variadic_mixed(1, 2, 3);
+ multi_variadic_mixed(1, 2, 3, 4);
}
// CHECK: "??$variadic_fn_template@@@YAXABH000@Z"
+// X64: "??$variadic_fn_template@@@YAXAEBH000@Z"
// CHECK: "??$variadic_fn_template@HHD$$BY01D@@YAXABH0ABDAAY01$$CBD@Z"
+// X64: "??$variadic_fn_template@HHD$$BY01D@@YAXAEBH0AEBDAEAY01$$CBD@Z"
+// CHECK: "??$multi_variadic_fn@HH$$ZHHH@@YAXH@Z"
+// X64: "??$multi_variadic_fn@HH$$ZHHH@@YAXH@Z"
+// CHECK: "??$multi_variadic_fn@HHH$$ZHH@@YAXH@Z"
+// X64: "??$multi_variadic_fn@HHH$$ZHH@@YAXH@Z"
+// CHECK: "??$multi_variadic_mixed@HHH$$V@@YAXHHH@Z"
+// X64: "??$multi_variadic_mixed@HHH$$V@@YAXHHH@Z"
+// CHECK: "??$multi_variadic_mixed@@@YAX@Z"
+// X64: "??$multi_variadic_mixed@@@YAX@Z"
template
struct VariadicClass {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D51784: ms: Insert $$Z in mangling between directly consecutive parameter packs.
This revision was automatically updated to reflect the committed changes.
Closed by commit rC341755: ms: Insert $$Z in mangling between directly
consecutive parameter packs. (authored by nico, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D51784?vs=164402&id=164577#toc
Repository:
rC Clang
https://reviews.llvm.org/D51784
Files:
lib/AST/MicrosoftMangle.cpp
test/CodeGenCXX/mangle-ms-templates.cpp
Index: test/CodeGenCXX/mangle-ms-templates.cpp
===
--- test/CodeGenCXX/mangle-ms-templates.cpp
+++ test/CodeGenCXX/mangle-ms-templates.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -fms-extensions
-fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -fms-extensions
-fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck -check-prefix
X64 %s
+// RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o -
-fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o -
-fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck
-check-prefix X64 %s
template
class Class {
@@ -185,13 +185,35 @@
// Unlike Itanium, there is no character code to indicate an argument pack.
// Tested with MSVC 2013, the first version which supports variadic templates.
-template void variadic_fn_template(const Ts &...args) { }
+template void variadic_fn_template(const Ts &...args);
+template
+void multi_variadic_fn(Ts... ts, Us... us);
+template
+void multi_variadic_mixed(Ts... ts, C c, Us... us);
void variadic_fn_instantiate() {
variadic_fn_template(0, 1, 3, 4);
variadic_fn_template(0, 1, 'a', "b");
+
+ // Directlly consecutive packs are separated by $$Z...
+ multi_variadic_fn(1, 2, 3, 4, 5);
+ multi_variadic_fn(1, 2, 3, 4, 5);
+
+ // ...but not if another template parameter is between them.
+ multi_variadic_mixed(1, 2, 3);
+ multi_variadic_mixed(1, 2, 3, 4);
}
// CHECK: "??$variadic_fn_template@@@YAXABH000@Z"
+// X64: "??$variadic_fn_template@@@YAXAEBH000@Z"
// CHECK: "??$variadic_fn_template@HHD$$BY01D@@YAXABH0ABDAAY01$$CBD@Z"
+// X64: "??$variadic_fn_template@HHD$$BY01D@@YAXAEBH0AEBDAEAY01$$CBD@Z"
+// CHECK: "??$multi_variadic_fn@HH$$ZHHH@@YAXH@Z"
+// X64: "??$multi_variadic_fn@HH$$ZHHH@@YAXH@Z"
+// CHECK: "??$multi_variadic_fn@HHH$$ZHH@@YAXH@Z"
+// X64: "??$multi_variadic_fn@HHH$$ZHH@@YAXH@Z"
+// CHECK: "??$multi_variadic_mixed@HHH$$V@@YAXHHH@Z"
+// X64: "??$multi_variadic_mixed@HHH$$V@@YAXHHH@Z"
+// CHECK: "??$multi_variadic_mixed@@@YAX@Z"
+// X64: "??$multi_variadic_mixed@@@YAX@Z"
template
struct VariadicClass {
Index: lib/AST/MicrosoftMangle.cpp
===
--- lib/AST/MicrosoftMangle.cpp
+++ lib/AST/MicrosoftMangle.cpp
@@ -1384,9 +1384,16 @@
assert(TPL->size() == TemplateArgs.size() &&
"size mismatch between args and parms!");
- unsigned Idx = 0;
- for (const TemplateArgument &TA : TemplateArgs.asArray())
-mangleTemplateArg(TD, TA, TPL->getParam(Idx++));
+ for (size_t i = 0; i < TemplateArgs.size(); ++i) {
+const TemplateArgument &TA = TemplateArgs[i];
+
+// Separate consecutive packs by $$Z.
+if (i > 0 && TA.getKind() == TemplateArgument::Pack &&
+TemplateArgs[i - 1].getKind() == TemplateArgument::Pack)
+ Out << "$$Z";
+
+mangleTemplateArg(TD, TA, TPL->getParam(i));
+ }
}
void MicrosoftCXXNameMangler::mangleTemplateArg(const TemplateDecl *TD,
Index: test/CodeGenCXX/mangle-ms-templates.cpp
===
--- test/CodeGenCXX/mangle-ms-templates.cpp
+++ test/CodeGenCXX/mangle-ms-templates.cpp
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
-// RUN: %clang_cc1 -std=c++11 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck -check-prefix X64 %s
+// RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=i386-pc-win32 | FileCheck %s
+// RUN: %clang_cc1 -std=c++11 -fms-compatibility-version=19 -emit-llvm %s -o - -fms-extensions -fdelayed-template-parsing -triple=x86_64-pc-win32 | FileCheck -check-prefix X64 %s
template
class Class {
@@ -185,13 +185,35 @@
// Unlike Itanium, there is no character code to indicate an argument pack.
// Tested with MSVC 2013, the first version which supports variadic templates.
-template void variadic_fn_template(const Ts &...args) { }
+template void variadic_fn_template(const Ts &...args);
+template
+void multi_variadic_fn(Ts... ts, Us... us);
+template
+void multi_variadic_mixed(Ts... ts, C c, Us... us);
void variadic_
r341756 - [Parser] Remove an unnecessary `mutable`
Author: maskray Date: Sat Sep 8 18:54:18 2018 New Revision: 341756 URL: http://llvm.org/viewvc/llvm-project?rev=341756&view=rev Log: [Parser] Remove an unnecessary `mutable` Modified: cfe/trunk/include/clang/Parse/Parser.h Modified: cfe/trunk/include/clang/Parse/Parser.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=341756&r1=341755&r2=341756&view=diff == --- cfe/trunk/include/clang/Parse/Parser.h (original) +++ cfe/trunk/include/clang/Parse/Parser.h Sat Sep 8 18:54:18 2018 @@ -119,7 +119,7 @@ class Parser : public CodeCompletionHand IdentifierInfo *Ident_pixel; /// Objective-C contextual keywords. - mutable IdentifierInfo *Ident_instancetype; + IdentifierInfo *Ident_instancetype; /// Identifier for "introduced". IdentifierInfo *Ident_introduced; ___ 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 🔧
stephanemoore updated this revision to Diff 164582.
stephanemoore added a comment.
Fix a couple issues:
• Forward declare `NSArray` in `objc-property-declaration.m`.
• Remove unnecessary lightweight generics declaration from `IDs` property in
`objc-property-declaration.m` to fix compilation error.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51832
Files:
clang-tidy/objc/PropertyDeclarationCheck.cpp
test/clang-tidy/objc-property-declaration-additional.m
test/clang-tidy/objc-property-declaration-custom.m
test/clang-tidy/objc-property-declaration.m
Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -1,7 +1,11 @@
// RUN: %check_clang_tidy %s objc-property-declaration %t
@class NSData;
+@class NSArray;
@class NSString;
@class UIViewController;
+@class CIColor;
+
+typedef void *CGColorRef;
@interface Foo
@property(assign, nonatomic) int NotCamelCase;
@@ -23,6 +27,9 @@
@property(assign, nonatomic) int enableGLAcceleration;
@property(assign, nonatomic) int ID;
@property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
@end
@interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN: [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}, \
-// RUN: {key: objc-property-declaration.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN: [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFShouldIncludeStandardAcronym;
-@end
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -34,91 +34,6 @@
CategoryProperty = 2,
};
-/// 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.
-constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
-"[2-9]G",
-"ACL",
-"API",
-"AR",
-"ARGB",
-"ASCII",
-"AV",
-"BGRA",
-"CA",
-"CF",
-"CG",
-"CI",
-"CRC",
-"CV",
-"CMYK",
-"DNS",
-"FPS",
-"FTP",
-"GIF",
-"GL",
-"GPS",
-"GUID",
-"HD",
-"HDR",
-"HMAC",
-"HTML",
-"HTTP",
-"HTTPS",
-"HUD",
-
[PATCH] D51832: [clang-tidy/checks] Update objc-property-declaration check to allow arbitrary acronyms and initialisms 🔧
stephanemoore updated this revision to Diff 164583.
stephanemoore added a comment.
Sorted forward declared classes in `objc-property-declaration.m`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D51832
Files:
clang-tidy/objc/PropertyDeclarationCheck.cpp
test/clang-tidy/objc-property-declaration-additional.m
test/clang-tidy/objc-property-declaration-custom.m
test/clang-tidy/objc-property-declaration.m
Index: test/clang-tidy/objc-property-declaration.m
===
--- test/clang-tidy/objc-property-declaration.m
+++ test/clang-tidy/objc-property-declaration.m
@@ -1,8 +1,12 @@
// RUN: %check_clang_tidy %s objc-property-declaration %t
+@class CIColor;
+@class NSArray;
@class NSData;
@class NSString;
@class UIViewController;
+typedef void *CGColorRef;
+
@interface Foo
@property(assign, nonatomic) int NotCamelCase;
// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'NotCamelCase' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
@@ -23,6 +27,9 @@
@property(assign, nonatomic) int enableGLAcceleration;
@property(assign, nonatomic) int ID;
@property(assign, nonatomic) int hasADog;
+@property(nonatomic, readonly) CGColorRef CGColor;
+@property(nonatomic, readonly) CIColor *CIColor;
+@property(nonatomic, copy) NSArray *IDs;
@end
@interface Foo (Bar)
Index: test/clang-tidy/objc-property-declaration-custom.m
===
--- test/clang-tidy/objc-property-declaration-custom.m
+++ /dev/null
@@ -1,18 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN: [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}, \
-// RUN: {key: objc-property-declaration.IncludeDefaultAcronyms, value: 0}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFIgnoreStandardAcronym;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'GIFIgnoreStandardAcronym' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(strong, nonatomic) NSString *TGIF;
-@end
Index: test/clang-tidy/objc-property-declaration-additional.m
===
--- test/clang-tidy/objc-property-declaration-additional.m
+++ /dev/null
@@ -1,15 +0,0 @@
-// RUN: %check_clang_tidy %s objc-property-declaration %t \
-// RUN: -config='{CheckOptions: \
-// RUN: [{key: objc-property-declaration.Acronyms, value: "ABC;TGIF"}]}' \
-// RUN: --
-@class NSString;
-
-@interface Foo
-@property(assign, nonatomic) int AbcNotRealPrefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:34: warning: property name 'AbcNotRealPrefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-// CHECK-FIXES: @property(assign, nonatomic) int abcNotRealPrefix;
-@property(assign, nonatomic) int ABCCustomPrefix;
-@property(strong, nonatomic) NSString *ABC_custom_prefix;
-// CHECK-MESSAGES: :[[@LINE-1]]:40: warning: property name 'ABC_custom_prefix' not using lowerCamelCase style or not prefixed in a category, according to the Apple Coding Guidelines [objc-property-declaration]
-@property(assign, nonatomic) int GIFShouldIncludeStandardAcronym;
-@end
Index: clang-tidy/objc/PropertyDeclarationCheck.cpp
===
--- clang-tidy/objc/PropertyDeclarationCheck.cpp
+++ clang-tidy/objc/PropertyDeclarationCheck.cpp
@@ -34,91 +34,6 @@
CategoryProperty = 2,
};
-/// 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.
-constexpr llvm::StringLiteral DefaultSpecialAcronyms[] = {
-"[2-9]G",
-"ACL",
-"API",
-"AR",
-"ARGB",
-"ASCII",
-"AV",
-"BGRA",
-"CA",
-"CF",
-"CG",
-"CI",
-"CRC",
-"CV",
-"CMYK",
-"DNS",
-"FPS",
-"FTP",
-"GIF",
-"GL",
-"GPS",
-"GUID",
-"HD",
-"HDR",
-"HMAC"
r341757 - Revert r341754.
Author: ahatanak
Date: Sat Sep 8 22:22:49 2018
New Revision: 341757
URL: http://llvm.org/viewvc/llvm-project?rev=341757&view=rev
Log:
Revert r341754.
The commit broke a couple of bots:
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/12347
http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-bootstrap/builds/7310
Modified:
cfe/trunk/include/clang/AST/Decl.h
cfe/trunk/include/clang/Sema/ScopeInfo.h
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/CodeGen/CGBlocks.cpp
cfe/trunk/lib/CodeGen/CGClass.cpp
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/Sema/ScopeInfo.cpp
cfe/trunk/lib/Sema/Sema.cpp
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Serialization/ASTReaderDecl.cpp
cfe/trunk/lib/Serialization/ASTWriterDecl.cpp
cfe/trunk/test/CodeGen/block-byref-aggr.c
cfe/trunk/test/CodeGen/blocks-seq.c
cfe/trunk/test/CodeGen/exceptions.c
cfe/trunk/test/CodeGen/personality.c
cfe/trunk/test/CodeGenCXX/block-capture.cpp
cfe/trunk/test/CodeGenCXX/blocks.cpp
cfe/trunk/test/CodeGenCXX/debug-info-blocks.cpp
cfe/trunk/test/CodeGenCXX/noescape.cpp
cfe/trunk/test/CodeGenObjC/arc-no-arc-exceptions.m
cfe/trunk/test/CodeGenObjC/arc-unoptimized-byref-var.m
cfe/trunk/test/CodeGenObjC/blocks-1.m
cfe/trunk/test/CodeGenObjC/noescape.m
cfe/trunk/test/CodeGenObjCXX/arc-blocks.mm
cfe/trunk/test/PCH/block-helpers.cpp
cfe/trunk/test/SemaObjCXX/blocks.mm
cfe/trunk/test/SemaObjCXX/noescape.mm
Modified: cfe/trunk/include/clang/AST/Decl.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Decl.h?rev=341757&r1=341756&r2=341757&view=diff
==
--- cfe/trunk/include/clang/AST/Decl.h (original)
+++ cfe/trunk/include/clang/AST/Decl.h Sat Sep 8 22:22:49 2018
@@ -965,8 +965,6 @@ protected:
/// Defines kind of the ImplicitParamDecl: 'this', 'self', 'vtt', '_cmd' or
/// something else.
unsigned ImplicitParamKind : 3;
-
-unsigned EscapingByref : 1;
};
union {
@@ -1409,19 +1407,6 @@ public:
NonParmVarDeclBits.PreviousDeclInSameBlockScope = Same;
}
- /// Indicates the capture is a __block variable that is captured by a block
- /// that can potentially escape (a block for which BlockDecl::doesNotEscape
- /// returns false).
- bool isEscapingByref() const;
-
- /// Indicates the capture is a __block variable that is never captured by an
- /// escaping block.
- bool isNonEscapingByref() const;
-
- void setEscapingByref() {
-NonParmVarDeclBits.EscapingByref = true;
- }
-
/// Retrieve the variable declaration from which this variable could
/// be instantiated, if it is an instantiation (rather than a non-template).
VarDecl *getTemplateInstantiationPattern() const;
@@ -3873,14 +3858,6 @@ public:
/// variable.
bool isByRef() const { return VariableAndFlags.getInt() & flag_isByRef; }
-bool isEscapingByref() const {
- return getVariable()->isEscapingByref();
-}
-
-bool isNonEscapingByref() const {
- return getVariable()->isNonEscapingByref();
-}
-
/// Whether this is a nested capture, i.e. the variable captured
/// is not from outside the immediately enclosing function/block.
bool isNested() const { return VariableAndFlags.getInt() & flag_isNested; }
Modified: cfe/trunk/include/clang/Sema/ScopeInfo.h
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ScopeInfo.h?rev=341757&r1=341756&r2=341757&view=diff
==
--- cfe/trunk/include/clang/Sema/ScopeInfo.h (original)
+++ cfe/trunk/include/clang/Sema/ScopeInfo.h Sat Sep 8 22:22:49 2018
@@ -31,7 +31,6 @@
#include "llvm/ADT/SmallVector.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/StringSwitch.h"
-#include "llvm/ADT/TinyPtrVector.h"
#include "llvm/Support/Casting.h"
#include "llvm/Support/ErrorHandling.h"
#include
@@ -203,12 +202,6 @@ public:
/// function.
SmallVector CompoundScopes;
- /// The set of blocks that are introduced in this function.
- llvm::SmallPtrSet Blocks;
-
- /// The set of __block variables that are introduced in this function.
- llvm::TinyPtrVector ByrefBlockVars;
-
/// A list of PartialDiagnostics created but delayed within the
/// current function scope. These diagnostics are vetted for reachability
/// prior to being emitted.
@@ -433,16 +426,6 @@ public:
(HasBranchProtectedScope && HasBranchIntoScope));
}
- // Add a block introduced in this function.
- void addBlock(const BlockDecl *BD) {
-Blocks.insert(BD);
- }
-
- // Add a __block variable introduced in this function.
- void addByrefBlockVar(VarDecl *VD) {
-ByrefBlockVars.push_back(VD);
- }
-
bool isCoroutine() const { return !FirstCo
