[PATCH] D114077: [clangd] Basic IncludeCleaner support for c/c++ standard library

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D114077#3218006 <https://reviews.llvm.org/D114077#3218006>, @thakis wrote:

> Breaks building on windows: http://45.33.8.238/win/51774/step_4.txt
>
> Ptal!

Fixed (I think) in a61f34ea2502d900c57a332174d4c103b6963c80 <https://reviews.llvm.org/rGa61f34ea2502d900c57a332174d4c103b6963c80>.
Clang successfully emulated MSVC's misunderstanding of the code :-(



================
Comment at: clang-tools-extra/clangd/Headers.h:330
+  static inline clang::clangd::stdlib::Header getEmptyKey() {
+    return clang::clangd::stdlib::Header(-1);
+  }
----------------
kbobyrev wrote:
> sammccall wrote:
> > kbobyrev wrote:
> > > maybe `DenseMapInfo<unsigned>::getEmptyKey()` and `DenseMapInfo<unsigned>::getTombstoneKey()` just like above?
> > empty/tombstone keys are reserved values, and we know what sensible reserved values are better than the traits for unsigned do. 
> > 
> > I can fix the code above if you like.
> Okay, that makes sense! Yes, that would be great if you could fix the code above for consistency :)
Done... yikes, the existing HeaderID empty key was zero... which is also the HeaderID for the main file!

Changed these to -1 and -2. Hopefully this is NFC, else maybe it fixed a scary bug...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114077