hokein added inline comments.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:115
+
+bool prefixedPropertyNameMatches(const llvm::StringRef &PropertyName,
+ const std::vector<std::string> &Acronyms) {
----------------
Wizard wrote:
> 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 :-)
Why? `MatchedDecl->getName()` returns `llvm::StringRef`. As the name indicates,
StringRef is a const reference of String, using StringRef is sufficient.
The same to ArrayRef. So the function is like `bool
prefixedPropertyNameMatches(llvm::StringRef PropertyName,
llvm::ArrayRef<std::string> Acronyms)`.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:151
+ hasCategoryPropertyPrefix(MatchedDecl->getName())) {
+ const auto *CategoryDecl = (const ObjCCategoryDecl *)(DeclContext);
+ if (!prefixedPropertyNameMatches(MatchedDecl->getName(), SpecialAcronyms)
||
----------------
Wizard wrote:
> 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
Sorry, I mean `llvm::dyn_cast` here, it should work.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:67
+ if (Style == CategoryProperty) {
+ for (size_t i = 0; i < Name.size(); ++i) {
+ if (Name[i] == '_') {
----------------
I think we save this code by using `find_first_of` and
`llvm::StringRef::tolower()` function.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:95
+// in matchers or not.
+std::string validPropertyNameRegex(const std::vector<std::string> &Acronyms,
+ bool UsedInMatcher) {
----------------
The same, ArrayRef.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:121
+
+bool hasCategoryPropertyPrefix(const llvm::StringRef &PropertyName) {
+ auto RegexExp = llvm::Regex("^[a-z]+_[a-zA-Z0-9][a-zA-Z0-9_]+$");
----------------
The same: `llvm::StringRef`
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:163
+ MatchedDecl->getName(),
+ llvm::ArrayRef<std::string>(SpecialAcronyms)) ||
+ CategoryDecl->IsClassExtension()) {
----------------
Do we need it? I think you can pass `SpecialAcronyms` directly.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42464
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
