.h files interpretted as C

[hedronvision]

.h files interpretted as C

[HaberR]

Hi! So I recently demoed this tool to some of my teammates and I've gotten a lot of love for it, and wanted to forward that your way! Daniel wrote me
"I've been writing C++ today and OMG it's so much better with the tool you showed off--actually checking BEFORE I compile"
So yeah, we're into it!

A few follow ups

• yes attaching to the container does tend to fix that issue, and I think that's exactly right about some of the stuff only being visible from within the container.
• I've noticed (and I might be wrong on this) that when a headerfile sets up a type-alias like so: using Vec = vector<T, Alloc<T>>; the alias tends not to be included properly in the consuming cc file. Can provide the exact error when I get in tomorrow.
• While we're not using Renovate yet, when I asked teammates about it there was definitely an appetite for it. The response seemed to be that we just needed some of our testing infrastructure to be a bit stronger before we felt ready for it.

[HaberR]

Thanks for getting back to me, I've confirmed that:

1. There is an entry for the header in compile_commands
2. Each line has the option --std=c++17 specified

Here's the clangd output when I open the file:
I[02:58:07.727] <-- textDocument/codeAction(225)
I[02:58:07.727] --> reply:textDocument/codeAction(225) 0 ms
I[02:58:07.727] --> textDocument/clangd.fileStatus
I[02:58:07.729] <-- textDocument/documentLink(226)
I[02:58:07.764] --> reply:textDocument/documentLink(226) 34 ms
I[02:58:07.766] --> textDocument/clangd.fileStatus

Not sure if it's any help, but the specific errors VSCode references are:
Unknown argument: '-fno-canonical-system-headers'clang(drv_unknown_argument)
Invalid argument '--std=c++17' not allowed with 'C'clang(drv_argument_not_allowed_with)

I can get the -fno-canonical-system-headers error to go away by deleting that from compile_commands, but deleting --std=c++17 seems to just push the issue further along (I get cstddef not found elsewhere).

[HaberR]

Amazing, it works!!! This is awesome, nobody should have to develop without autocomplete. I do still see the squiggle for -no-canonical-system-headers but that's small potatoes. Thanks for sharing this project, and for the incredible support!

[HaberR]

I work for Third Wave Automation, we're building autonomous forklifts!

[cpsauer]

Hey, @HaberR! Thanks for giving the tool a try. Sorry things aren't working as expected.

As background:
This is caused by a clangd issue that generally plagues tooling, but a good chunk of the code of this tool (unlike others) is there to work around it.

We also have files like this internally. That behavior is super annoying, so we spent time trying to make this case work for everyone. Sorry that isn't working for you; there must be a subcase we hadn't considered. Let's figure out where things are going wrong!

How the fix is intended to work:
We extract the headers used (transitively) by a specific source file and then additionally emit compile_commands.json entries for the headers, marking them as being compiled the same way as their source files.

It's be great to figure out where in the process things are going wrong. Could I ask for your help? (I'm assuming there isn't public code I can play around with?)

For a given .h file that's giving us trouble:

1. Is there indeed an entry for that header in compile_commands.json?
   [Confirms that the fix is trying to apply itself to the header.]
2. Is there a language specifier in the command associated to the entry?
   For example: -std=gnu++17
   [Hypothesis: There won't be, and this is the problem. The project doesn't specify an explicit language standard, so that isn't applied to the header. We missed this case, because we, and the other test projects, explicitly set the language standard.]
3. If there isn't a language specifier, does adding one solve the issue?
   Quick test: Add one manually and save. Recent clangd versions should automatically pick up the change on save and refocusing the file--and tell you so in their debug output.
   Interim fix if this works--but we'll get to it quickly: You can temporarily add, e.g., build --cxxopt=-std=gnu++17 to your .bazelrc, to specify the language standard, and then refresh compile_commands.json.
4. If there is already a language specifier or 3 doesn't work, it's be great to make sure clangd is actually picking up the compile_commands.json.
   We want to look at the clangd output logs. In VSCode, you can get to them through [Terminal Tab]>Output>[Dropdown]>clangd.
   Then open up the header in question. Hopefully the log has an entry something like "ASTWorker building file with command <...>"

Anyway, lmk! I'll also run some tests over here shortly.

[cpsauer]

Yeah, of course. Thanks for digging in. That that really narrowed down the problem.

I can reproduce the issue! Or at least the Invalid argument '--std=c++17' not allowed with 'C' clang(drv_argument_not_allowed_with) part. (On my machine, C++ completion does seems to work nicely on my machine for those files, though.)

Further, I can solve it by manually specifying the language, inferring it from the extension from the source file that included it! I've just pushed up a commit that does just that.

(In the background: Clangd 13 seems to detect the language from the file name, assume all .h files are C, and then ignore any evidence in the command to the contrary.)

Woo. Okay! So could you let me know if that fixes things for you?

[cpsauer]

[I don't anticipate that commit fixing the -no-canonical-system-headers error message, since that's pretty different. From a quick Google, it looks like that's a GCC-specific flag that clang doesn't support. I think clangd would gracefully recover, but if it breaks things or gets annoying, lmk. We could automatically strip it out of compile_commands.json via refresh.template.py::_all_platform_patch]

[cpsauer]

Huzza! Delighted to hear it, @HaberR. Good autocomplete makes me smile, too :)

Thank you for bringing this up and for working with me to narrow it down and fix it. It'll help out a lot of others, too.

Please let me know if there are other rough edges you see--or if you have ideas on how to make things even better.

-Chris

[cpsauer]

Hey so one other update: The (awesome) clangd folks are going to fix this case, after some discussion. See https://reviews.llvm.org/D116167 for more.

When it's released, I'll test and then revert my workaround to keep things clean and simple.

[cpsauer]

@HaberR, could I ask for your help?

In a sec, I'm going to revert the workaround, since clangd 14 was just released, which should contain their fix for the underlying problem.

Could I ask you to quickly update (both this tool and clangd) and make sure that the issue doesn't come back? I've tested quickly myself, but I'd love it if you'd double check. Plus, there are a lot of good fixes in the latest :)

-Chris

[HaberR]

Hi, sorry for the delay, just seeing this, thanks for checking in!

I am actually bumping into this issue again.. I'm wondering if maybe it's because I didn't execute:

code --install-extension llvm-vs-code-extensions.vscode-clangd
# We also need make sure that Microsoft's C++ extension is not involved and interfering.
code --uninstall-extension ms-vscode.cpptools

I'm running my code server on a remote machine and only connecting to it locally--I don't think those instructions would work for my case?

At the top of my .h files I'm getting the following error:

Invalid argument '--std=c++17' not allowed with 'C'clang(drv_argument_not_allowed_with)

Running clangd v0.1.17, and compile-commands-extractor af9af15f7bc16fc3e407e2231abfcb62907d258f

[cpsauer]

Hey, @HaberR! Thanks for reaching back out.

Trying to scope: Sounds like the error is back for you now that I reverted the workaround--clangd having released a fix to the underlying issue. I'm assuming that this reemerged when you updated this tool to include the commit that undid the workaround. Is that right?

Could I first ask you to confirm that you're using the latest clangd (>=14)? (That is, at least version 14 of clangd the binary--not just the latest vscode extension.) You can double check this by looking at the top of clangd's output in VSCode (View>Output, then select clangd from the dropdown). And if clangd is too old, you can update by opening the command palette and typing "clangd check for update".

Sorry for the hassle; just want to make sure that the solution isn't as simple as upgrading clangd.

[I'm afraid I haven't used all this with a remote dev environment, so I'm not sure how clangd works with all that. But if it worked before, then it seems more likely that the problem is the above.]

[HaberR]

gah, yup wrong clangd binary, upgrading that fixed it. Thank you, and sorry for the trouble!

[cpsauer]

Woo! Great. Good you wrote in--and glad it's working.

While we're chatting, anything special users should know if they're trying to set things up for remote dev? Or does it just work out of the box?

I'm also curious how you're approaching updating this tool in your WORKSPACE--just so I can keep tabs on what folks are doing.

[And if there's anything else feedback wise that I should know, really, please say! I'd love to hear it.]

[HaberR]

Don't remember needing to do anything special--I think it just worked right out of the box!

Re our usage: In order to make the setup a little easier for the rest of my team, I've added the clangd config to our settings.json and similarly configured our extensions.json. Things also tend to work a bit better when I use this in conjunction with the 'attach to running container' functionality to connect to our dev environment. A big part of that comes from the fact that when I run from outside of that container, <nlohmann/json.hp> gives me

In included file: 'filesystem' file not foundclang(pp_file_not_found)

and that doesn't happen from within the container. Unfortunately that single issue can throw a good chunk of the type-checking off (also unfortunate is the fact that we include that library in lots of places).

I appreciated some of the new features (automatic symlinks, more readable compile_commands.json, automatic .gitignore update). My only complaint is that it can be a touch slow for the intellisense to kick in when I open a new cc file (3 seconds?) but that's a small price to pay!

I know the recommendation is to use Renovate for the purpose of updating our dependencies, but I don't think we have any such tool in place... At the moment I think our upgrade strategy is to do it when someone notices a new feature that they want (or when they're forced to) 😬

[cpsauer]

Sweet. Thank you so much @HaberR for your unfiltered feedback there. It really helps improve my mental model of how you (and others!) use this.

Sounds like with the attach you manage to get around that issue? (Please lmk if wrong--not sure I'm totally sure what's going on, but I'm guessing it has to do with referencing STL headers at locations only found inside the container.)

On the speed: Pretty sure that's clangd's indexing speed, sadly. (We're just generating the compile_commands.json, which I'd like to make faster, too!) CCLS is a bit faster, apparently, but I think clangd is probably the right call long term.

And I appreciate your honestly about Renovate! While I'd recommend it, I really appreciate your telling me that you don't use it. No need to feel bad. In fact, incredibly valuable to help me get a sense of how people are actually approaching updates.

Thanks again!
Chris

[cpsauer]

Hey, I'm so glad. Puts a big smile on my face that this is helping you guys. Thanks for forwarding :) I'm curious what you all are building together with it!