[PATCH] D42464: add prefix with '_' support for property name. Corresponding apple dev doc: https://developer.apple.com/library/content/qa/qa1908/_index.html

[Yan Zhang via Phabricator via cfe-commits]

Wizard added inline comments.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:93
+                 [](const std::string &s) { return llvm::Regex::escape(s); });
   // Allow any of these names:
   // foo
----------------
hokein wrote:
> Does the comment still make sense? Seems you changed the regex below.
Yes they are actually the same. I am just replacing the prefix "::" with "^" according to the UsedInMatcher param.


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:115
+
+bool prefixedPropertyNameMatches(const llvm::StringRef &PropertyName,
+                                 const std::vector<std::string> &Acronyms) {
----------------
hokein wrote:
> no need to pass const reference of `llvm::StringRef`. `StringRef` is cheap.
The original property name I directly get from ast is a const. If I remove the const here, I will have to make a copy of the property name before calling this.
I prefer keep this const to save that copy :-)


================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:151
+      hasCategoryPropertyPrefix(MatchedDecl->getName())) {
+    const auto *CategoryDecl = (const ObjCCategoryDecl *)(DeclContext);
+    if (!prefixedPropertyNameMatches(MatchedDecl->getName(), SpecialAcronyms) ||
----------------
hokein wrote:
> Consider using `const auto* CategoryDecl = dyn_cast<ObjCCategoryDecl*>(DeclContext)`, we can get rid of this cast, and `NodeKind` variable.
Tried that before but I encountered 2 issues:
1. 'clang::DeclContext' is not polymorphic
2. cannot use dynamic_cast with -fno-rtti
which are preventing me from using dynamic_cast


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D42464