[PATCH] D21603: [include-fixer] Add missing namespace qualifiers after inserting a missing header.
Daniel Jasper via cfe-commits
cfe-commits at lists.llvm.org
Wed Jul 6 06:27:08 PDT 2016
djasper added inline comments.
================
Comment at: include-fixer/IncludeFixer.cpp:166
@@ -159,1 +165,3 @@
QueryString = ExtendNestedNameSpecifier(Range);
+ SymbolRange = tooling::Range(SM.getDecomposedLoc(Range.getBegin()).second,
+ QueryString.size());
----------------
Lots of code duplication. Maybe pull out function/lambda?
================
Comment at: include-fixer/IncludeFixer.cpp:259
@@ +258,3 @@
+ // Query the symbol based on C++ name Lookup rules.
+ // Firstly, lookup the identifier with scoped namespace contexts; If fails,
+ // falls back to look up the identifier directly.
----------------
"If that fails, .."
================
Comment at: unittests/include-fixer/IncludeFixerTest.cpp:144
@@ -141,1 +143,3 @@
+ runIncludeFixer("a::b::foo bar;\n",
+ /*FixNamespaceQualifiers=*/true, IncludePath));
----------------
I think we should set this to true everywhere or more precisely completely remove this parameter. Now that the flag is gone, we are starting to test implementation details IMO.
http://reviews.llvm.org/D21603
More information about the cfe-commits
mailing list