[PATCH] D143509: Move the BySpelling map to IncludeStructure.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang-tools-extra/clangd/Headers.cpp:303
+  llvm::SmallVector<Inclusion> Includes;
+  auto It = MainFileIncludesBySpelling.find(Spelling);
+  if (It == MainFileIncludesBySpelling.end())
----------------
VitaNuo wrote:
> kadircet wrote:
> > nit:
> > ```
> > llvm::SmallVector<Inclusion*> Includes;
> > for (auto Idx : MainFileIncludesBySpelling.lookup(Spelling))
> >   Includes.push_back(&MainFileIncludes[Idx]);
> > return Includes;
> > ```
> But `lookup` will return 0 if it doesn't find the spelling in the map (default value of `int`). And at the same time 0 is a valid index. Isn't that just wrong?
`MainFileIncludesBySpelling` is a map with values of type vector, not int. hence it'll be an empty vector, not `0`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143509/new/

https://reviews.llvm.org/D143509