[PATCH] D46374: Add support for ObjC property name to be a single acronym.

Ben Hamilton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri May 4 08:17:53 PDT 2018


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


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:222
+                  [MatchedDecl](std::string const &s) {
+                    auto Acronym  = llvm::Regex("^" + s + "$");
+                    return Acronym.match(MatchedDecl->getName());
----------------
Please be aware this will change the match from running a single regular expression to running ~ 70 regular expressions on every single `@property`. I would expect this to perform pretty poorly.



================
Comment at: test/clang-tidy/objc-property-declaration.m:24
 @property(assign, nonatomic) int enableGLAcceleration;
+ at property(assign, nonatomic) int ID;
 @end
----------------
Wizard wrote:
> benhamilton wrote:
> > Please add a test for a built-in regex (4G) as well as a custom regex in the other test file.
> Unable to add single property test of 4G because it is illegal to use digit as the first character of property name.
Ah, of course.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D46374





More information about the cfe-commits mailing list