klimek added inline comments.
================
Comment at: include/clang/Tooling/Core/Lookup.h:37-38
@@ +36,4 @@
+/// \param FromDecl The declaration to which the nested name points.
+/// \param ReplacementString The replacement nested name. Should be fully
+/// qualified.
+/// \returns The new name to be inserted in place of the current nested name.
----------------
Don't most tests violate the "should be fully qualified" requirement here?
================
Comment at: lib/Tooling/Core/Lookup.cpp:60
@@ +59,3 @@
+
+ // Otherwise strip of redundant namespace qualifications from the new name.
+ std::string NS =
----------------
s/of/off/
================
Comment at: lib/Tooling/Core/Lookup.cpp:63-66
@@ +62,6 @@
+ cast<NamespaceDecl>(DeclA)->getQualifiedNameAsString() + "::";
+ if (NewName.startswith("::"))
+ NS = "::" + NS;
+ if (NewName.startswith(NS))
+ return NewName.substr(NS.size());
+ DeclA = DeclA->getParent();
----------------
If NewName is fully qualified, why would we ever want to allow that to match
from a non-global namespace?
namespace a { namespace b { namespace c {
void foo();
}}}
-> if NewName is "::c::bar" (for whatever reason), this should probably not
silently go to bar inside 'c', or to c::bar inside 'b'.
================
Comment at: lib/Tooling/Core/Lookup.cpp:72
@@ +71,3 @@
+/// Check if the name specifier begins with a written "::".
+static bool isNameSpecifierGlobal(const NestedNameSpecifier *NNS) {
+ while (NNS) {
----------------
Isn't that more like isFullyQualified then?
================
Comment at: unittests/Tooling/LookupTest.cpp:19-20
@@ +18,4 @@
+ bool VisitCallExpr(CallExpr *Expr) {
+ if (OnCall)
+ OnCall(Expr);
+ return true;
----------------
Is this ever not true?
http://reviews.llvm.org/D13931
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
