[PATCH] D43869: [clangd] Support include canonicalization in symbol leve.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.


================
Comment at: clangd/index/CanonicalIncludes.cpp:83
+  static const std::vector<std::pair<const char *, const char *>> SymbolMap = {
+      // Map symbols in <iosfwd> to their preferred includes.
+      {"std::basic_filebuf", "<fstream>"},
----------------
hokein wrote:
> Looks like the list only contains stream-related symbols, there are some symbols in `<iosfwd>` not covered, is it intended? 
> 
> Maybe we keep the order of this map the same as the one listed in http://en.cppreference.com/w/cpp/header/iosfwd? That would make it easier to see the difference?
Although `allocator` could be exposed by <iosfwd>, it's not defined in <iosfwd>, so we don't need to handle it specially.

I am sorting the symbols by their canonical headers, which I think would make it easier to check the header mapping.


================
Comment at: unittests/clangd/SymbolCollectorTests.cpp:561
+      class no_map {};
+      class ios {};
+      class ostream {};
----------------
hokein wrote:
> The STL implementation of `ios` is a typedef `typedef basic_ios<char>  ios; `, I think we should make the test align with it?
The symbol mapping doesn't make a difference between symbol types, and symbol kind test is not in the scope of this patch,  so I think this wouldn't be necessary here.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43869