[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