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

[Viktoriia Bakalova via Phabricator via cfe-commits]

VitaNuo marked 2 inline comments as done.
VitaNuo 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())
----------------
kadircet wrote:
> 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`.
Oh right. Sorry I guess I got confused.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143509