Wizard marked 3 inline comments as done.
Wizard added inline comments.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:115
+
+bool prefixedPropertyNameMatches(const llvm::StringRef &PropertyName,
+ const std::vector<std::string> &Acronyms) {
----------------
hokein wrote:
> 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)`.
>
>
If I remove the const in the method, compiler will have error of "candidate
function not viable: expects an l-value for 1st argument". I believe it cannot
accept a const reference as arg if the definition is var.
================
Comment at: clang-tidy/objc/PropertyDeclarationCheck.cpp:151
+ hasCategoryPropertyPrefix(MatchedDecl->getName())) {
+ const auto *CategoryDecl = (const ObjCCategoryDecl *)(DeclContext);
+ if (!prefixedPropertyNameMatches(MatchedDecl->getName(), SpecialAcronyms)
||
----------------
hokein wrote:
> 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.
It is not working either. It says "error: static_cast from 'clang::Decl *' to
'const clang::ObjCCategoryDecl *const *' is not allowed" though I have no idea
why it is regarded as a static cast.
I am using it like this:
const auto *CategoryDecl = llvm::dyn_cast<const ObjCCategoryDecl
*>(DeclContext);
================
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_]+$");
----------------
hokein wrote:
> The same: `llvm::StringRef`
Cannot remove the const for the same reason.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42464
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
