Open amosnier opened 4 years ago
amosnier
commented
4 years ago If anybody has the issue above, a workaround that seems to work is to place a C++ file "close to" the header file in the file hierarchy, for instance in the same directory, or to move the header file "closer" to some C++ files in the the file hierarchy. In my case, the header file is a template declaration, which happens to be "quite far" from the closest C++ file (i.e. there are some C-files that are "closer").
gislan
commented
4 years ago When choosing a file to infer the commands from, we always prefer one in the same language, but we only consider some candidates, so if there are no C++ files nearby and no C++ files has identical name, we'll choose a C file instead.
https://reviews.llvm.org/D87253 makes it so that if there's even one C++ file in the project, that one would be preferred. I'm honestly not sure if that's the right thing to do or not. Let's see what others think.
kadircet
commented
4 years ago (i've posted in review, but also posting it in here, in case someone wants to implement one of the proposed solutions)
As discussed offline, this is trading off some accuracy between getting -I correct vs -std and it is unclear whether that's beneficial or harmful. It is easy to come up with examples for both and we were split 50/50 between each.
In the end all of this is a heuristic work and it is quite likely that we might break this for more people, while trying to fix this one specific case. So I don't think it's worth it.
As for suggestions(again from offline discussion) :
bradlarsen
commented
4 years ago We can implement a more sophisticated transfer logic in clangd layer, by making use of compile commands from a TU including a particular header
Note, it appears that this is what CLion does. Perhaps also vscode. (EDIT: By "vscode" I mean Microsoft's C/C++ extension for vscode.)
amosnier
commented
4 years ago We can implement a more sophisticated transfer logic in clangd layer, by making use of compile commands from a TU including a particular header
With my limited knowledge of clangd, that makes perfect sense to me. That's kind of the point of this issue. It can be impossible to guess what context the user expects when opening a header file, but that header file will typically be included in at least one of the TU handled by the compilation database. In such a case picking one of these contexts would be much better than guessing.
Note, it appears that this is what CLion does. Perhaps also vscode. (EDIT: By "vscode" I mean Microsoft's C/C++ extension for vscode.)
Out of curiosity, how do you know what CLion does? Is that public information?
bradlarsen
commented
4 years ago Note, it appears that this is what CLion does. Perhaps also vscode. (EDIT: By "vscode" I mean Microsoft's C/C++ extension for vscode.)
Out of curiosity, how do you know what CLion does? Is that public information?
There is this bit in the CLion documentation about compiling single files (the emphasis is mine):
Although the build functionality for compilation database projects is not yet implemented in CLion, you may find it useful to check the changes in a single file without building the whole project. For this purpose, CLion provides the Recompile action. It is available for individual source and header files, and also for groups of files selected in the project tree. For headers, CLion uses resolve context to compile one of the source files that include the specified header.
Now, recompilation is not quite the same thing as deducing compile database commands for a header file, but is a similar problem.
Note also that I haven't done any actual experimentation with CLion here, and have only been going by documentation, so perhaps I've spoken too soon.
bradlarsen
commented
4 years ago We can implement a more sophisticated transfer logic in clangd layer, by making use of compile commands from a TU including a particular header
For posterity / future readers: another approach / workaround to the issue of clangd improperly guessing compilation commands for a header file is to include an explicit entry for it in the compilation database. So you sidestep the entire task of asking clangd to guess the compile flags.
compdb supports a workflow of taking an existing compilation database and enriching it with entries for header files in the project. I have just used this very briefly, and it seems to work so far. It looks like compdb uses a variation of the find-compilation-units-that-include-the-given-header approach.
amosnier
commented
4 years ago @bradlarsen, thanks, really interesting reads. I was just trying to write down some thinking about header files in compilation databases when you posted your entry about compdb. My thinking was that given the objectives of compilation databases, it would not have seemed entirely crazy to extend the file format to include translation units dependencies.
Like:
{ "directory": "/home/user/llvm/build",
"command": "/usr/bin/clang++ -Irelative -DSOMEDEF=\"With spaces, quotes and \\-es.\" -c -o file.o file.cc",
"file": "file.cc"
"deps": ["file.h", "file2.h"] }or something similar.
That would seem better than making up compilation commands for header files when they do not really exist. And clangd's (and other's!) job would be much simpler. Because of course, any proper build system has that information, since it needs it to ensure that TU only get recompiled when updated.
I have also found this CMake issue, which I guess is relevant.
valeros
commented
4 years ago Hi guys, is there any news on this issue? I'm trying to use the VSCode extension in pair with a GCC cross-compiler for an embedded target. My project contains several files with code that rely on compiler type and since headers are not present in the compilation database Clang is trying to pretend to be the default compiler in the OS (in my case it MSVC) which leads to intellisense issues. It's somewhat possible to partially overcome the problem with clangd.fallbackFlags, but then Clangd is not able to find standard headers since --query-driver is not taken into account.
HighCommander4
commented
4 years ago Clangd should still be picking some source file from the compilation database to infer compile commands from for a header.
The issue in this ticket, is that it sometimes infers a source file of the wrong language (e.g. a C source file for a C++ header in a project that contains a mix of source files in both languages). Is that the case for your project?
If your issue is that clangd is not inferring a source file at all, that's a different issue that can potentially be solved on your end. (For example, if the header is outside the directory containing the compilation database, clangd won't use the CDB for it by default, but it can be made to with the --compile-commands-dir option.)
If you share a clangd log (from VSCode's Output view, "Clangd Language Server" option in the dropdown) that should shed some light on things.
HighCommander4
commented
4 years ago (Updated issue title to reflect more specifically what this issue is about.)
njames93
commented
4 years ago Could we not use a dynamic database(saved on disc, next to the index perhaps) that stores the best match for headers/sources. Could even run when indexing. The basic idea is track where the headers are included, and sort (as you go) based on if its included in a file with a close name match, If it's the first include in the file, If it's included in a source file or another header file. When it comes to infer the compile command for the file, query that index and use the compile command from the best source match. If it's a header that is only included in other headers, infer from one of the other included headers.
valeros
commented
4 years ago Hi @HighCommander4! It's partially related to this issue. I don't have any C++ files in my CDB and still Clangd is trying to parse an external header file as a C++ header with fallback flags. Here are parts from the verbose log:
As you can see with main.c file, Clangd correctly used standard headers from the provided query driver, but with an out-of-tree header file it used C++ flags. Please let me know if you would like to see the entire log.
HighCommander4
commented
4 years ago I don't have any C++ files in my CDB and still Clangd is trying to parse an external header file as a C++ header with fallback flags. Here are parts from the verbose log: [...] As you can see with
main.cfile, Clangd correctly used standard headers from the provided query driver, but with an out-of-tree header file it used C++ flags.
In your case, the issue is not that clangd is picking a wrong-language file from the CDB, but rather that it's not using the CDB for out-of-tree headers at all.
For each open file, clangd looks for a CDB in the directory containing the file and its ancestor directories. If your CDB is in your workspace root and you open a file outside the workspace root, this algorithm therefore will not find the CDB for such files.
You can force clangd to use a CDB for all files include out-of-tree files by passing --compile-commands-dir=<directory> as a command-line argument to clangd, where <directory> is the directory containing your CDB.
HighCommander4
commented
4 years ago @njames93 Yes, I think a heuristic based on actual inclusion relationships would be the "proper" solution here.
stelonix
commented
3 years ago Facing the same issue, hpp files seem to be treated as C files by clangd on vscode. Is there any way to force .hpp files to be treated as C++ files?
cpsauer
commented
3 years ago Ran into this same thing, and ended up introspecting the build system (Bazel) to generate compile command entries for all the headers, too. I just made headers use the same commands as the sources that use them.
Seems to me, though, like clangd should try to do the same automatically and implicitly from a compile_commands file that contains only sources. Header-only libraries are a common thing in C++, and it's easy to trip this case.
Also, it seems to me that the original title of this issue was better; seems like a more general heuristic issue that applies to inferring all flags from compilation context, not just languages.
cpsauer
commented
3 years ago While looking at solving this another way, I ran across the -M (--dependencies) clang flag, which dumps the headers used by a particular file. It really seems to me like the right solution is to have clangd infer commands for headers from the source files that use them! Perhaps we could mostly reuse infrastructure from that flag
Falven
commented
3 years ago I don't have any C++ files in my CDB and still Clangd is trying to parse an external header file as a C++ header with fallback flags. Here are parts from the verbose log: [...] As you can see with
main.cfile, Clangd correctly used standard headers from the provided query driver, but with an out-of-tree header file it used C++ flags.In your case, the issue is not that clangd is picking a wrong-language file from the CDB, but rather that it's not using the CDB for out-of-tree headers at all.
For each open file, clangd looks for a CDB in the directory containing the file and its ancestor directories. If your CDB is in your workspace root and you open a file outside the workspace root, this algorithm therefore will not find the CDB for such files.
You can force clangd to use a CDB for all files include out-of-tree files by passing
--compile-commands-dir=<directory>as a command-line argument toclangd, where<directory>is the directory containing your CDB.
What about for situations where we do not have access to the compilation unit/cpp file?
Even with this change, it seems clangd still tries to use the fallback command and arguments when parsing a file in the CBD that includes a header not in the CDB. It isn't until you open the header that clangd tries to use the nearest compilation unit metadata for compilation.
kadircet
commented
3 years ago Even with this change, it seems clangd still tries to use the fallback command and arguments when parsing a file in the CBD that includes a header not in the CDB. It isn't until you open the header that clangd tries to use the nearest compilation unit metadata for compilation.
sorry i don't follow the argument. when parsing a file clangd fetches the compile flags for that specific file and it has nothing to do with what headers are included in that file. could you elaborate ?
amosnier
commented
3 years ago For the record, this issue has indeed nothing to do with cases where no CDB can be found for a source code file. As far as I am concerned, I either have a CDB file in the top directory of my source file tree, or I use --compile-commands-dir= to point to the correct CDB directory (in the case of a source code tree with multiple targets, for instance). In my experience, that works very well.
If one opens a file for which no CDB can be found, one cannot complain that clangd uses some fallback flags.
In the case I reported, clangd did find a CDB, but it did not find a rule for the opened file in it, because the opened file was a header file. Clangd's heuristic still did a pretty good job at finding an alternate file in the CDB, in order to collect some applicable compiler flags, but I was unlucky, because there was no file of the right language "close to" that header file.
That's why I am hoping for an even better heuristic. I understand that it is not a trivial problem and that clangd developers do not want to take the risk of releasing an "improvement" that would actually be harmful for some users.
I still like the suggestion of
making use of compile commands from a TU including a particular header
and hope it can become reality.
HighCommander4
commented
3 years ago In the case I reported, clangd did find a CDB, but it did not find a rule for the opened file in it, because the opened file was a header file. Clangd's heuristic still did a pretty good job at finding an alternate file in the CDB, in order to collect some applicable compiler flags, but I was unlucky, because there was no file of the right language "close to" that header file.
That's why I am hoping for an even better heuristic. I understand that it is not a trivial problem and that clangd developers do not want to take the risk of releasing an "improvement" that would actually be harmful for some users.
I still like the suggestion of
making use of compile commands from a TU including a particular header
and hope it can become reality.
+1, please see #123 for a recent improvement in this area, and discussions of possible further improvements.
amosnier
commented
3 years ago @HighCommander4, thanks for the link, that allowed me to find (again) compdb, which will be useful in the short term. I had just ran into this issue again. It's unfortunately really easy. At work, I get this problem because I develop embedded software. At home, I'm playing with OpenGL, doing all my development in C++, but Glad for instance is C-code. So my auto-generated C++ header files containing GLSL source code get interpreted as C-code. Ironically, the first thing clangd complains about is that it does not recognize the symbol namespace... And ironically too, it did seem that the first JSON message generated by clangd for the same file had a mention of the C++-language. Anyway, still looking forward to a better solution.
Nahor
commented
3 years ago I'm having the same problem but with cross-compilation.
I'm compiling a Yocto project. One of my file includes C++ "vector" header. In my IDE (vscode), it opens the correct file (the header used by Yocto's cross-compiler, i.e. ".../
cpsauer
commented
2 years ago Heads up for those taking the approach of enriching compile_commands.json with entries for all the headers files, listing the command that compiles them:
clangd13 will infer that all .h files are C, even if the command associated with them is for a .cpp file. This leads to errors and completion issues. You can resolve this by detecting and manually specifying the --language flag based on the file extension. (You need to do similar tricks to solve the same problem for objc.)
(For the context that brought this up and our workaround, see https://github.com/hedronvision/bazel-compile-commands-extractor/issues/12)
kadircet
commented
2 years ago Heads up for those taking the approach of enriching compile_commands.json with entries for all the headers files, listing the command that compiles them: clangd13 will infer that all .h files are C, even if the command associated with them is for a .cpp file.
Sorry I don't follow the issue here. If compile_commands.json is enriched to have entries for header files, clangd will no longer "infer" compile flags for those files but rather make use of whatever is mentioned in the database. Are you sure the description/diagnosis of the issue is correct here?
sam-mccall
commented
2 years ago I think the issues are:
clang -std=c++11 foo.h would parse as C++, but it's an error instead (in both clang and clangd)Neither are strictly bugs (this is consistent with the compile command model) but it's fairly unfriendly.
cpsauer
commented
2 years ago Ah, sorry. Slightly different case. Some clarification and a concrete example:
Lets say we have foo.h imported by foo.cpp, and that foo.cpp (and thus also the code in foo.h) is compiled with:
clang -std=c++11 foo.cpp (note foo.cpp, not .h)
One even more obvious attempt to fix is to emit an entry to compile_commands.json for foo.h, using the same command.
{
"file": "foo.h",
"command": "clang -std=c++11 foo.cpp"
...This will then lead to an error like Invalid argument '--std=c++11' not allowed with 'C' clang(drv_argument_not_allowed_with)
This makes me think that clangd effectively makes the file swap in the command @sam-mccall describes above, which then makes the command invalid. This can be solved by detecting that the original file is .cpp, and adding, e.g. --language=c++, to the (already valid) command.
Sorry for the lack of clarity. And, as usual, thanks @sam-mccall for stepping in to make things better. Figured I should say something, having just worked through it in response to another issue, since others using the solutions described here might run into the same.
sam-mccall
commented
2 years ago Oh thanks for the example! I forgot about the case where file doesn't match the argv.
Maybe we can reasonably detect that case and apply the "transfer" logic, which already exists and corrects flags to account for file extensions.
sam-mccall
commented
2 years ago https://reviews.llvm.org/D116167 should address this recent issue raised by @cpsauer: explicitly enriching the database by simply copying the argv should work.
Regarding other ideas discussed here:
So I think we should maybe-fix the second bullet (to be discussed in review) and then close this bug.
cpsauer
commented
2 years ago Wow. Thank you, @sam-mccall. That was fast and super well worded. Has anyone told you recently that you're awesome?
(One curiosity question: It's not obvious to me why there is a separate -x c++-header from just normal -x c++. If you know why, I'd love to hear! But no pressure if not.)
sam-mccall
commented
2 years ago @cpsauer You're too kind :-)
why there is a separate -x c++-header
This is a pretty minor distinction.
-Wunused apply in header files but not main files, clang warns on #include_next in the main file, I've landed D116167, so now this bug is just about the original topic: preferring (more) files from the same language.
cpsauer
commented
2 years ago Sweet! Makes sense. Thanks, @sam-mccall.
Looking forward to removing the workaround just as soon as clangd14 ships!
simark
commented
2 years ago I would like to submit an additional use case related to this. I have header files with preprocessor conditions, meaning the active portions of the header files vary depending on which file includes it. I would like to be able to tell clangd: show me the analysis for this header file as seen by this source file or that source file, and be able to switch at runtime.
I imagine a flow like this:
Would that make sense?
HighCommander4
commented
2 years ago @simark I think that would be really neat. It's probably worth tracking in a different issue as it's more involved (e.g. it likely involves a new LSP protocol, and client-side UI).
Whether or not this qualifies as a bug report or a feature request could I guess be discussed.
After reporting https://llvm.discourse.group/t/header-file-heuristics-issue/1749 and investigating it further, I arrive at the conclusion that the issue I am describing is probably just a logical consequence of the clangd heuristics used for header files in my project structure.
A short summary of the issue:
.hpp.Updating file /path/to/Header.hpp with command inferred from /other/path/to/arbitrary.c. I.e. clangd picks a C-file instead of a C++ file.Sometimes, however, clangd will pick a C++ file which does not necessarily include the header file in question. While I understand that picking the right translation unit for a header file is a little arbitrary (there could be several correct answers, with different compiler flags), it feels like picking a C++ file instead of a C file in the case above should be feasible, and could also be a non-negligible improvement for some uses cases (like mine, because I would then get the correct flags, like
-std=c++17etc.). It seems like the heuristics use some kind of distance in the file hierarchy between the header file and the C/C++ file inferred as a criterion, which I guess is fine as a general principle, but it would be nice if files of the same language as the header file's (if clangd knows about that, but in my case it is clear from the extension) are picked with a higher priority.