[PATCH] D118755: [clangd] Crash in __memcmp_avx2_movbe

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks! This fix makes sense to me.



================
Comment at: clang-tools-extra/clangd/test/repeated_includes.test:1
+# RUN: rm -rf %/t
+# RUN: mkdir -p %t && touch %t/t.h && touch %t/t2.h && touch %t/t3.h
----------------
This should be tested more directly rather than via clangd, e.g. rather in `clang/unittests/Tooling/HeaderIncludesTest.cpp`


================
Comment at: clang/include/clang/Tooling/Inclusions/HeaderIncludes.h:100
   // and "x" will be treated as the same header when deleting #includes.
   llvm::StringMap<llvm::SmallVector<Include, 1>> ExistingIncludes;
 
----------------
An alternative would be to use a std::forward_list<Include> here.

This guarantees pointer stability, it's an extra allocation but seems unlikely to matter.

It would be more robust if the data structure changes (e.g. becomes large, or is mutated after creation) but probably none of this will ever happen.

Up to you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D118755