bazel-compile-commands-extractor icon indicating copy to clipboard operation
bazel-compile-commands-extractor copied to clipboard

clangd does not include C++ system headers in the search path for files with a ".h" extension

Open NomiChirps opened this issue 2 years ago • 7 comments

In my C++ project the file naming convention is foo.cc for source files and foo.h for headers, which I think is not too unusual. I was puzzled about why clangd seemed to be unable to find any system headers (e.g. #include <cassert>) only when looking at my header files -- the source files worked just fine. One exciting journey through the clangd source code later, I discovered that it extracts a different set of system include paths for C- and C++-language files (naturally), but, in the absence of a -x gcc flag specifying it explicitly, will guess the language based on the file's extension.

I was able to work around this issue without renaming my header files by adding --cxxopt=-xc++ to the refresh_compile_commands() rule in my BUILD file, like so:

refresh_compile_commands(
    name = "refresh_compile_commands",
    targets = {
        "//myproject/...": "--cxxopt=-xc++",
    },
)

This was a big headache to track down, though. Would it make sense for bazel-compile-commands-extractor to always add this flag when running bazel aquery? If not, maybe a note in the FAQ :)

NomiChirps avatar Jul 25 '22 05:07 NomiChirps

Hey, @NomiChirps! Good to hear from you again, and thanks for your contributions.

Could I ask you to check that this is happening even with the latest version of clangd? (14.0.3 at the time of writing.)

For context: We used to automatically add such a language flag for headers with ambiguous extensions, based on the extension of the corresponding source. Clangd then fixed the underlying issue in 14.0.0, so we removed the workaround. (This is consistent with your reading of the code, because the source file, not the header, is being supplied in the command.) The full historical context on this issue is in #12. But if this issue is back somehow, I definitely want to put the language flag logic back in.

Cheers, Chris

cpsauer avatar Jul 25 '22 08:07 cpsauer

Aha, I think I see. Thanks for those pointers to the clangd issue; I wasn't aware of the transferCompileCommand behavior added in https://reviews.llvm.org/D116167.

It looks like the root issue might actually be with my toolchain definition; I'm using https://github.com/dfr/rules_pico with --platforms=@rules_pico//platforms:pico.

So far I've found that, with a minimal cc_library() test target and a standard Bazel gcc-on-windows config (set BAZEL_SH to the msys2 bash.exe and put --compiler=mingw-gcc in .bazelrc), the resulting compile_commands.json contains only a single entry, for my "test.cc", and everything works as it should.

But with the rules_pico toolchain, it contains entries not only for "test.cc", but also "test.h" and for the entire transitive dependency set of system headers (g:\\code\\tools\\gcc-arm-11.2-2022.02-mingw-w64-i686-arm-none-eabi\\arm-none-eabi\\include\\machine\\ieeefp.h and so on).

No doubt what's happening is that clangd skips the flag-transfer logic in https://reviews.llvm.org/D116167 because a command line for "test.h" is provided explicitly in compile_commands.json.

I'm trying to figure out what exactly it is about the rules_pico toolchain definition that causes this difference in bazel-compile-command-extractor's behavior. Will let you know if I find anything.

NomiChirps avatar Jul 25 '22 20:07 NomiChirps

Thanks for diving in, @NomiChirps!

To confirm, if you run something like bazel aquery "mnemonic('(Objc|Cpp)Compile',deps(//...))", you're seeing entries for headers with this rules_pico toolchain, but not with a standard toolchain!? If so, very strange indeed that they'd be creating compile actions for the headers. We could, I suppose, add a filter for malformed compile actions like that, but it seems offhand like the toolchain should be fixed to not create them. [I'm assuming you're passing exclude_headers = "all" to this tool, so we'd expect no headers in compile_commands.json. I'm also assuming that by "entries for test.h", you mean that the command arguments list contains "test.h", not just that the file is "test.h", so as to not invoke that flag-transfer logic.]

cpsauer avatar Jul 26 '22 08:07 cpsauer

Sorry, I should have been more clear! No, the compile_commands.json looks like this: (full gist)

[
  {
    "file": "test.cc",
    "arguments": [
      "G:/code/tools/gcc-arm-11.2-2022.02-mingw-w64-i686-arm-none-eabi/bin/arm-none-eabi-gcc",
      "-U_FORTIFY_SOURCE",
      "-D_FORTIFY_SOURCE=1",
      "-DFREESTANDING",
      "-fstack-protector",
      "-Wall",
      "-fno-omit-frame-pointer",
      "-mcpu=cortex-m0plus",
      "-mthumb",
      "-MD",
      "-MF",
      "bazel-out/x64_windows-fastbuild/bin/_objs/test/test.d",
      "-frandom-seed=bazel-out/x64_windows-fastbuild/bin/_objs/test/test.o",
      "-iquote",
      ".",
      "-iquote",
      "bazel-out/x64_windows-fastbuild/bin",
      "-no-canonical-prefixes",
      "-Wno-builtin-macro-redefined",
      "-D__DATE__=\"redacted\"",
      "-D__TIMESTAMP__=\"redacted\"",
      "-D__TIME__=\"redacted\"",
      "-c",
      "test.cc",
      "-o",
      "bazel-out/x64_windows-fastbuild/bin/_objs/test/test.o"
    ],
    "directory": "G:/code/compile-commands-issue-repro"
  },
  {
    "file": "g:\\code\\tools\\gcc-arm-11.2-2022.02-mingw-w64-i686-arm-none-eabi\\arm-none-eabi\\include\\machine\\endian.h",
    "arguments": [
      "G:/code/tools/gcc-arm-11.2-2022.02-mingw-w64-i686-arm-none-eabi/bin/arm-none-eabi-gcc",
      [... same flags ...]
      "-c",
      "test.cc",
      "-o",
      "bazel-out/x64_windows-fastbuild/bin/_objs/test/test.o"
    ],
    "directory": "G:/code/compile-commands-issue-repro"
  },
  [... many more system headers ...]
  [eventually there is also a 'file: "test.h"' with the same command line]

bazel aquery "mnemonic('(Objc|Cpp)Compile',deps(//...))" only produces one command, for test.cc, as expected.

I haven't been setting exclude_headers at all! I skimmed over that part of the README, oops :). I've been assuming headers should have been excluded by default. With exclude_headers = "all", it behaves as expected: just one item in compile_commands.json, for test.cc, and clangd guesses the header language correctly. Testing now, I can't seem to reproduce the behavior I mentioned before with the mingw-gcc toolchain, of the headers being excluded even without exclude_headers being specified. Not sure what happened there; probably a mistake. Sorry for the red herring!

Okay, so, observations:

Toolchain compile_commands.json contains headers? clangd guesses header language correctly? Reproduction compile_commands.json gist
rules_pico Yes No https://github.com/NomiChirps/compile-commands-issue-repro https://gist.github.com/NomiChirps/834c67898314a4d4a53ddebbb481fce8
mingw-gcc Yes/No ??? Yes https://github.com/NomiChirps/compile-commands-issue-repro/tree/mingw-gcc https://gist.github.com/NomiChirps/7a2228daca8a6a00b6c9414076c9b65b
MSVC (Bazel default) Yes Yes https://github.com/NomiChirps/compile-commands-issue-repro/tree/msvc https://gist.github.com/NomiChirps/0c5edf05bca533c5e131100295638704

The relevant difference seems to be that the mingw-gcc toolchain specifies --std=gnu++0x, but rules_pico specifies neither --std nor -x. However, MSVC doesn't specify /TP or anything either, so now I'm extra confused about clangd's logic here.

Thank you for working with me to figure this out!

edit: Okay, weird. Now the mingw-gcc toolchain DOES exclude headers again, even without exclude_headers=all. Random, inconsistent behavior? There must be something I'm not controlling for in my tests.

edit2: I should mention that I've been testing mostly with clangd-14.0.3, but my original issue also occurred with 14.0.0 and with clangd_snapshot_20220717.

NomiChirps avatar Jul 26 '22 15:07 NomiChirps

Clangd verbose log for rules_pico: https://gist.github.com/NomiChirps/85afddb252bb9699b2d0d59a2a3f43f3

For mingw-gcc; however, I can no longer reproduce the compile_commands.json in the table above. This log is with this compile_commands.json, which excludes all header files for some unknown reason. https://gist.github.com/NomiChirps/7421c7029fada1b0634caf9e887e73f8

For MSVC: https://gist.github.com/NomiChirps/d8d662f5cd8a2209cc7837d561326542

Curiously, clangd's system include extraction fails for both mingw-gcc and MSVC, but clangd uses the system MSVC headers in both cases. Apparently a fallback.

NomiChirps avatar Jul 26 '22 16:07 NomiChirps

Thanks for scoping things some more, @NomiChirps.

Since I'm traveling--and a few thousand miles from my Windows machines--it's going to be several days before I can get back to this properly. Sorry to not have my usual speed, but I wanted to at least give you a heads up.

A couple of things in the meantime, if you want:

  1. If you notice anything more or come up with any more hypotheses, please let me know.
  2. It might be worth trying to run via the command line, the command from the arguments list for test.h in compile_commands.json. The goal would be making sure it is well formed and runs cleanly. If so, the underlying issue is probably a clangd bug, which we should also file over at clangd/clangd. If so it also might be worth temporarily trying ccls to see if it can avoid the problem.

Cheers! Sorry to be out of town--and have this not be smooth sailing for you.

Chris

cpsauer avatar Jul 28 '22 22:07 cpsauer

I am having the same issue when compiling Tensorflow on a Mac M1 pro. Clangd treated the header files as C files until I added the "--cxxopt=-xc++" flag to refresh_compile_commands. With the "--cxxopt=-xc++" flag, it works correctly, treating header files as C++.

js8544 avatar Jul 29 '22 13:07 js8544

Hey guys. Sorry it's taken me so much longer than usual to get to this. But I'm back and working on it.

cpsauer avatar Aug 20 '22 04:08 cpsauer

My first attempts to reproduce were on my mac, but I'm not managing to hit the error case yet, even without -x or -std.

For example, I tried grabbing the sample repo, stripped down to be toolchain-independent (since I didn't have the pico compiler installed), but removed all references to -std in the generated compile_commands.json. (There were no -x, language entries.) I was seeing similar results on my other test repos, so clearly I haven't figured out what's causing the issue. That's probably why it's there :)

When I looked at the clangd output (In VSCode, open the terminal pane -> output tab -> clangd from dropdown, while test.h is open) I'm seeing that clangd is doing the correct thing, auto inserting -x c++-header to prevent the issue, consistent with https://reviews.llvm.org/D116167.


I'll switch to my windows desktop shortly, but I'm hoping I can get a bit more of your help tracking things down and getting us to a minimal reproduction. The goal is to get a crisp understanding of what's wrong so that we can both patch around it and worth with the clangd/clangd folks to resolve the underlying issue

@NomiChirps, could I ask you to try manually adding -std=c++11 to the test.h compile_commands.json in the error case, and confirm that that fixes the error case? Knowing that we can intentionally trip the issue will go a long way towards being sure we know the cause.

You may have to manually restart clangd for things to take effect. (VSCode command palate: CTRL+shift+P -> clangd:restart language server) If adding -std doesn't work, could you try with -x c++-header? And then could I ask you to look at the clangd output to see what command it was choosing, with and without the addition? Search for the line after "test.h version 1 with command" in the logs.

And @js8544, any further ideas on how to reduce your tensorflow case into a more simple, minimal example? I was trying on my mac, but again, things seemed to work with clangd 14, even without any -x or -std flags.

Thanks for your help and patience tracking this down! Chris

cpsauer avatar Aug 20 '22 04:08 cpsauer

Or to try to track things down from the other end: Does the following, super minimal example, modified from @NomiChirps', reproduce the issue for you, @js8544, or you, @NomiChirps, if you use pico or gcc and play with the flags? I'm try to track down whether its -x and/or -std flags that trigger this on your systems, since that isn't triggering it on mine, or whether it's something about a specific toolchain.

clangd_system_headers.zip

(Just download and bazel run :refresh_compile_commands)

I'm seeing the working behavior above, with or without -std. My compile_command.json looks like the following:

Click to expand
[
  {
    "file": "test.cc",
    "arguments": [
      "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang",
      "-D_FORTIFY_SOURCE=1",
      "-fstack-protector",
      "-fcolor-diagnostics",
      "-Wall",
      "-Wthread-safety",
      "-Wself-assign",
      "-fno-omit-frame-pointer",
      "-O0",
      "-DDEBUG",
      "-std=c++11",
      "-iquote",
      ".",
      "-iquote",
      "bazel-out/darwin-fastbuild/bin",
      "-MD",
      "-MF",
      "bazel-out/darwin-fastbuild/bin/_objs/test/test.d",
      "-frandom-seed=bazel-out/darwin-fastbuild/bin/_objs/test/test.o",
      "-isysroot",
      "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk",
      "-F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks",
      "-F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/Library/Frameworks",
      "-no-canonical-prefixes",
      "-pthread",
      "-no-canonical-prefixes",
      "-Wno-builtin-macro-redefined",
      "-D__DATE__=\"redacted\"",
      "-D__TIMESTAMP__=\"redacted\"",
      "-D__TIME__=\"redacted\"",
      "-target",
      "x86_64-apple-macosx12.3",
      "-c",
      "test.cc",
      "-o",
      "bazel-out/darwin-fastbuild/bin/_objs/test/test.o"
    ],
    "directory": "/Users/cpsauer/Downloads/compile-commands-issue-repro-main"
  },
  {
    "file": "test.h",
    "arguments": [
      "/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang",
      "-D_FORTIFY_SOURCE=1",
      "-fstack-protector",
      "-fcolor-diagnostics",
      "-Wall",
      "-Wthread-safety",
      "-Wself-assign",
      "-fno-omit-frame-pointer",
      "-O0",
      "-DDEBUG",
      "-std=c++11",
      "-iquote",
      ".",
      "-iquote",
      "bazel-out/darwin-fastbuild/bin",
      "-MD",
      "-MF",
      "bazel-out/darwin-fastbuild/bin/_objs/test/test.d",
      "-frandom-seed=bazel-out/darwin-fastbuild/bin/_objs/test/test.o",
      "-isysroot",
      "/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk",
      "-F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/System/Library/Frameworks",
      "-F/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/Library/Frameworks",
      "-no-canonical-prefixes",
      "-pthread",
      "-no-canonical-prefixes",
      "-Wno-builtin-macro-redefined",
      "-D__DATE__=\"redacted\"",
      "-D__TIMESTAMP__=\"redacted\"",
      "-D__TIME__=\"redacted\"",
      "-target",
      "x86_64-apple-macosx12.3",
      "-c",
      "test.cc",
      "-o",
      "bazel-out/darwin-fastbuild/bin/_objs/test/test.o"
    ],
    "directory": "/Users/cpsauer/Downloads/compile-commands-issue-repro-main"
  }
]

cpsauer avatar Aug 20 '22 05:08 cpsauer

Also, @NomiChirps, anything I should know to efficiently install that compiler for pico?

cpsauer avatar Aug 20 '22 05:08 cpsauer

And @NomiChirps, I was going through your clangd log for the problem case, and I'm seeing it auto adding the -x c++-header in the command for test.h, meaning that it does think it's a c++ header, right? Any other theories about what's going on here?

cpsauer avatar Aug 20 '22 05:08 cpsauer

Or to try to track things down from the other end: Does the following, super minimal example, modified from @NomiChirps', reproduce the issue for you, @js8544, or you, @NomiChirps, if you use pico or gcc and play with the flags? I'm try to track down whether its -x and/or -std flags that trigger this on your systems, since that isn't triggering it on mine, or whether it's something about a specific toolchain.

clangd_system_headers.zip

(Just download and bazel run :refresh_compile_commands)

I'm seeing the working behavior above, with or without -std. My compile_command.json looks like the following:

Click to expand

It does reproduce for me. Clangd gives Invalid argument '-std=c++0x' not allowed with 'C' and 'cstdint' file not found on test.h but no error on test.cc

js8544 avatar Aug 20 '22 10:08 js8544

Oh I see. My system has clang 13.0.1, which doesn't include the fix @cpsauer mentioned above. So recognizing headers as c files is expected.

js8544 avatar Aug 20 '22 10:08 js8544

Just to be clear, clangd 13, right? If so, good news, since clangd 14 should resolve that issue. (Seems likely that we're dealing with two different root causes here.)

cpsauer avatar Aug 20 '22 10:08 cpsauer

Just to be clear, clang_d_ 13, right? If so, good news, since clangd 14 should resolve that issue. (Seems likely that we're dealing with two different root causes here.)

Yep, clangd 13. So my case is closed :)

js8544 avatar Aug 20 '22 10:08 js8544

Yay! Great. Delighted to hear we've got things working for you. One down, one to go.

cpsauer avatar Aug 20 '22 20:08 cpsauer

@NomiChirps, thinking about your clangd log:

Do those detected default include paths (e.g. g:\code\tools\gcc-arm-11.2-2022.02-mingw-w64-i686-arm-none-eabi\bin\../lib/gcc/arm-none-eabi/11.2.1/include) look generally right? Like if you resolve them on your machine via CMD, do they point somewhere valid?

[Looks like cstdint (the missing header in test.h) is at g:\\code\\tools\\gcc-arm-11.2-2022.02-mingw-w64-i686-arm-none-eabi\\arm-none-eabi\\include\\c++\\11.2.1\\cstdint. That differs by a bit, but is close enough that maybe there's junctioning. Interestingly, the c++-specific paths don't always appear as auto-detected in my (working) macOS setup either, so that particular part doesn't seem to be a cause for alarm.]

It's seeming more and more likely (from the log) that clangd is recognizing the right language, since it's explicitly inferring -x c++-header for cl. But I'm not sure how to reconcile that with --cxxopt=-xc++ fixing things for you.

cpsauer avatar Aug 24 '22 01:08 cpsauer

@HighCommander4, sorry to pull you in, but any idea what might be going on from the clangd side here?

cpsauer avatar Aug 24 '22 01:08 cpsauer

Okey dokey, here's as clean and complete of a reproduction as I think I can manage: https://github.com/NomiChirps/compile-commands-testing

There's no rules_pico or alternate toolchain involved, and I'm now on Linux instead of Windows. Just one cc_binary and one cc_library, 3 files in total. Nevertheless clangd/vscode is having trouble with the include in "foo.h" unless I add the --cxxopt=-xc++ workaround. I've also included a copy of all the logs I can think of. I'll try to dig into it when I have time and see if I can find anything obviously wrong.

I should also note that another functioning workaround seems to be setting exclude_headers="all", which I'm now using in my main project instead of "--cxxopt=-xc++", because of the positive effect it has on VSCode's performance. exclude_headers="external" without "--cxxopt=xc++" does NOT work.

Please let me know if this reproduces the issue on your system too!

NomiChirps avatar Aug 24 '22 04:08 NomiChirps

Hey @NomiChirps! Thanks so much

...but, and you're not going to like this, I'm earnestly trying and haven't yet managed to reproduce. All works fine so far on my machine, with the logs showing it's (re)loaded the database, and inferred that it's a C++ header, found iostream. All like your bad log, except that it doesn't then choke on the transitive include. Obviously substantial system differences--like that mine uses clang under the hood instead of gcc.

[The great irony here, of course, is that all this header entry emitting is to work around, basically, the exact issue you initially suspected this was: inferring the wrong language for .h files. exclude_headers="all" will definitely cause that problem in some cases, for example, if you directly open a C++ header-only library that has files in another language (C or Objc) right closest to it.]

I'll dig in more tm (downloading and spinning up a new VM overnight), but I'm guessing this is going to end up being an underlying clangd bug--perhaps with how it navigates gcc's default includes. If the compile_commands.json looks right, and the clangd output wrong, then we've got to go a layer down into clangd. Hence my trying to call in @HighCommander4 for his clangd expertise. In my experience, he's been magically good at tracing these. And then we can get it over to clangd/clangd, understanding what's going wrong.

-CS

cpsauer avatar Aug 24 '22 06:08 cpsauer

Yay! I can reproduce the issue with GCC in this new Ubuntu VM. (GCC 11.2.0, so it reproduces with a different gcc version than yours.)

That facilitated my distilling the error case down further to something independent of bazel and this project, so the clangd folks can easily play with it. Thanks @NomiChirps for your patience in helping me reproduce the issue. Sorry about my clang-based-blindness, the issue, and how long this has taken.

cpsauer avatar Aug 25 '22 03:08 cpsauer

@NomiChirps, it's unrelated to this problem, but I happened to notice that your logs indicate your clangd configuration is missing the compile-commands-dir flag from the README. I figured I should give you a friendly heads, since it's useful to have, e.g., so that clangd correctly finds commands for external or generated code. Again, no impact on this particular issue--just noticed and figured you'd want the heads up.

cpsauer avatar Aug 25 '22 03:08 cpsauer

Being able to play around with the problem let me significantly narrow down the issue. Here's what I found:

  • The problem had to do with include_next; changing the problematic, transitive #include_next <cstdlib>s to #include <cstdlib> makes the problem go away. Similarly, directly including cstdlib from foo.h works fine.
  • As you'd said, things resolve if you add -xc++ to the header. But no problems appear if you add -xc++-header to the source.
  • Removing the entry for the header causes the error to go away, even though that means clangd is inferring the same command from the other file.
  • Changing to a hpp extension causes the error to go away.
  • Disabling --query-driver causes the error to go away.

I'll submit the issue to the clangd folks in a sec, but please do say something if you were seeing something more general. I know that in the very first post you were saying that all stdlib includes were showing erroneous red squiggles, which is different from what I was seeing.

cpsauer avatar Aug 25 '22 04:08 cpsauer

Okay, hopefully that's report thoroughly captures what we know on the simplest of cases, so they can solve this problem a layer down.

In the meantime, let's get an interim fix. I think the cleanest is to restore the -x inserting workaround, since I know of another clangd bug that could benefit from our re-inserting that.

cpsauer avatar Aug 25 '22 05:08 cpsauer

Restored that workaround with improvements!

Should fix things until the clangd folks do, so I'm going to optimistically close this for now.

But please check that it fixes things for you! And if it doesn't, holler, and I'll open this right back up.

cpsauer avatar Aug 25 '22 07:08 cpsauer

That fixes it well enough for me! Thank you for spending so much of your time chasing this down :)

NomiChirps avatar Aug 25 '22 22:08 NomiChirps

You're very welcome. Sorry it took so long.

You say 'well enough'. Is there something still broken? If so, I'd like to know!

Chris

cpsauer avatar Aug 26 '22 04:08 cpsauer

Thank you for looking at this! I won't be able to check until Monday but I'll definitely come back and take another look then.

Meanwhile -- I don't think it has anything to do with win/mac/linux, it's definitely the rules_pico toolchain. What I've done as another brute force workaround in the meantime is to use a jq filter to remove all the .h files' and system include files' "compile commands" from compile_commands.json, leaving only the commands for my project's .cc files, as I think it's supposed to be. This made vscode completion much more responsive as it no longer has to slog through the ENORMOUS compile_commands.json that gives a command for every system header...

On Fri, Aug 19, 2022, 10:22 PM Christopher Sauer @.***> wrote:

And @NomiChirps https://github.com/NomiChirps, I was going through your clangd log for the problem case https://gist.github.com/NomiChirps/85afddb252bb9699b2d0d59a2a3f43f3, and I'm seeing it auto adding the -x c++-header in the command.

— Reply to this email directly, view it on GitHub https://github.com/hedronvision/bazel-compile-commands-extractor/issues/70#issuecomment-1221235708, or unsubscribe https://github.com/notifications/unsubscribe-auth/AQWIKDOW7FSTJ7WA7SZUIE3V2BTR5ANCNFSM54Q5HYOQ . You are receiving this because you were mentioned.Message ID: @.*** .com>

NomiChirps avatar Oct 11 '22 08:10 NomiChirps