alexfh requested changes to this revision.
This revision now requires changes to proceed.
================
Comment at: clang-rename/USRFindingAction.cpp:49
@@ -47,4 +48,3 @@
public:
- explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context,
- std::vector<std::string> *USRs)
- : FoundDecl(FoundDecl), Context(Context), USRs(USRs) {}
+ explicit AdditionalUSRFinder(const Decl *FoundDecl, ASTContext &Context)
+ : FoundDecl(FoundDecl), Context(Context) {}
----------------
Why explicit?
================
Comment at: clang-rename/USRFindingAction.cpp:140
@@ +139,3 @@
+public:
+ explicit NamedDeclFindingConsumer(
+ ArrayRef<unsigned> SymbolOffsets, ArrayRef<std::string> QualifiedNames,
----------------
What's the reason to make the constructor explicit?
================
Comment at: clang-rename/USRFindingAction.cpp:158
@@ -150,1 +157,3 @@
+
+ if (QualifiedName.empty())
FoundDecl = getNamedDeclAt(Context, Point);
----------------
Use conditional operator.
================
Comment at: clang-rename/USRFindingAction.h:23
@@ -19,1 +22,3 @@
+
+using llvm::ArrayRef;
----------------
No using declarations in headers, please. Also, since your code is in the clang
namespace, you can include clang/Basic/LLVM.h that re-declares ArrayRef in
namespace clang.
================
Comment at: clang-rename/USRFindingAction.h:38
@@ -31,5 +37,3 @@
- // \brief get the spelling of the USR(s) as it would appear in source files.
- const std::string &getUSRSpelling() { return SpellingName; }
-
- const std::vector<std::string> &getUSRs() { return USRs; }
+ const std::vector<std::string> &getUSRSpellings() { return SpellingNames; }
+ const std::vector<std::vector<std::string>> &getUSRList() { return USRList; }
----------------
Return ArrayRef.
================
Comment at: clang-rename/USRFindingAction.h:39
@@ -36,1 +38,3 @@
+ const std::vector<std::string> &getUSRSpellings() { return SpellingNames; }
+ const std::vector<std::vector<std::string>> &getUSRList() { return USRList; }
----------------
Same here.
https://reviews.llvm.org/D24224
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
