[PATCH] D30720: [include-fixer] Add fuzzy SymbolIndex, where identifier needn't match exactly.

Haojian Wu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 8 09:01:24 PST 2017


hokein added inline comments.


================
Comment at: include-fixer/FuzzySymbolIndex.cpp:138
+    return llvm::errorCodeToError(Buffer.getError());
+  return std::unique_ptr<FuzzySymbolIndex>(new MemSymbolIndex(
+      find_all_symbols::ReadSymbolInfosFromYAML(Buffer.get()->getBuffer())));
----------------
nit: `llvm::make_unique`.


================
Comment at: include-fixer/FuzzySymbolIndex.h:54
+} // namespace clang
+#endif
----------------
nit: an empty line before `#endif` and trailing `// LLVM_CLANG_TOOLS_EXTRA_INCLUDE_FIXER_FUZZY_SYMBOL_INDEX_H`.


================
Comment at: include-fixer/SymbolIndexManager.cpp:106
       // Match the identifier name without qualifier.
-      if (Symbol.getName() == Names.back()) {
-        bool IsMatched = true;
----------------
Just want to verify: this judgement is not needed as it is guaranteed in Line 96 `DB.get()->search(Names.back())` right?


================
Comment at: include-fixer/SymbolIndexManager.cpp:156
   for (const auto &SymAndSig : MatchedSymbols)
     Res.push_back(std::move(SymAndSig.Symbol));
   return Res;
----------------
An off-topic catch: std::move should not work for `const auto &`, remove the `const`.


================
Comment at: include-fixer/tool/ClangIncludeFixer.cpp:233
+          return std::move(*DB);
+        });
+  }
----------------
I'd prefer to add a `break;` statement although this case is the last one. 


https://reviews.llvm.org/D30720





More information about the cfe-commits mailing list