[PATCH] D54872: [clangd] Add ability to not use -resource-dir at all

[Marc-Andre Laperle via Phabricator via cfe-commits]

malaperle added a comment.

In D54872#1319958 <https://reviews.llvm.org/D54872#1319958>, @ilya-biryukov wrote:

> I'm a bit confused now, will put up a few clarifying questions to make sure I understand the probem properly.
>  Does `clang-cl` work correctly the arguments from `compile_commands.json`? Does it fail if we add the `-resource-dir`, similar to how it was done for clangd?


Yes it does fail the same way if I use clang-cl from the same local build as the clangd with the added -resource-dir. Interestingly, it does also fail if I don't add the -resource-dir, unlike clangd (this patch). For that clang-cl to work on the command-line, I have to use -resource-dir C:\Program Files\LLVM\lib\clang\8.0.0\include. The clang-cl from the installer works without the -resource-dir. Presumably clang-cl might also be adding the resource-dir silently like Clangd?

> The `compile-commands.json` seems to be generated for the MSVC  or hand-crafted, I wonder if it may be a difference between the clang frontend and msvc (unlikely, but just to be sure).

Sorry, I confused things because I changed the generator to output "cl" instead of "clang-cl" just as a test. I wrote the generator so you can say that I hand-crafted the generator? ;) Anyway, I've seen no difference between using cl or clang-cl in the compile_commands.json

> The `compile_commands.json` does not even mention the `-resource-dir`, I wonder why would adding it break anything? The only thing that comes to mind is some of the microsoft headers being named the same as the clang headers, but having more stuff in them (the `uintptr_t` that's missing). Is that the case?

Here is the preprocessor output when I run the "win installer" clang-cl (no -resource-dir):

  # 48 "C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\Professional\\VC\\Tools\\MSVC\\14.15.26726\\include\\vcruntime.h" 2 3
  # 1 "C:\\Program Files\\LLVM\\lib\\clang\\8.0.0\\include\\vadefs.h" 1 3
  # 32 "C:\\Program Files\\LLVM\\lib\\clang\\8.0.0\\include\\vadefs.h" 3  !!!! #include_next <vadefs.h> !!!!
  # 1 "C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\Professional\\VC\\Tools\\MSVC\\14.15.26726\\include\\vadefs.h" 1 3
  ...
  typedef unsigned __int64 uintptr_t;

Here is the preprocessor output when I run the custom-built clang-cl (no -resource-dir, but with /I"C:\Program Files\LLVM\lib\clang\8.0.0\include"):

  # 48 "C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\Professional\\VC\\Tools\\MSVC\\14.15.26726\\include\\vcruntime.h" 2 3
  # 1 "C:\\Program Files\\LLVM\\lib\\clang\\8.0.0\\include\\vadefs.h" 1 3
  # 32 "C:\\Program Files\\LLVM\\lib\\clang\\8.0.0\\include\\vadefs.h" 3 !!!! #include_next <vadefs.h> !!!!
  # 1 "D:\\git\\llvm\\build-vs2017\\RelWithDebInfo\\lib\\clang\\8.0.0\\include\\vadefs.h" 1 3
  # 33 "C:\\Program Files\\LLVM\\lib\\clang\\8.0.0\\include\\vadefs.h" 2 3
  # 49 "C:\\Program Files (x86)\\Microsoft Visual Studio\\2017\\Professional\\VC\\Tools\\MSVC\\14.15.26726\\include\\vcruntime.h" 2 3
  ...
  (MSVC's vadefs.h never included)

So the #include_next in the presence of two vadefs.h from different Clang messes it up. It seems wrong to have this include in the compile commands: /I"C:\Program Files\LLVM\lib\clang\8.0.0\include", right? Because it will always clash with any other Clang's internal headers. I'll check if we can remove that flag in the build or if there was some special reason it was there that could impact the solution for the problem we're trying to solve here.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D54872