Repository navigation
auto-populate the browse.path when compileCommands is used #1715
Description
I try to get use compile commands json, but I do not get it to work. I have included a minimal repro project files inline below.
Repro steps - case 1 - define:
- Open foo/main.c (and have no other files open)
- Hover mouse over RETURN_VALUE
- Right click and "Go to Definition"
Result I observe:
- Message "No definition found for 'RETURN_VALUE'"
Expected result:
- Editor opens bar/header.h
I also cannot use Go to Definition to navigate to the header file. If header.h is already open, then Go to Definition works.
- Extension Name: cpptools
- Extension Version: 0.15.0
- OS Version: Linux x64 4.13.0-37-generic
- VSCode version: 1.21.1
We have written the needed data into your clipboard. Please paste:
{
"activationTimes": {
"startup": false,
"codeLoadingTime": 123,
"activateCallTime": 7,
"activateResolvedTime": 526,
"activationEvent": "onLanguage:c"
}
}.vscode/c_cpp_properties.json:
{
"configurations": [
{
"name": "Linux",
"compileCommands": "${workspaceRoot}/compile_commands.json",
"includePath": [],
"defines": [],
"intelliSenseMode": "clang-x64",
"browse": {
"path": [],
"limitSymbolsToIncludedHeaders": false,
"databaseFilename": ""
}
}
],
"version": 3
}.vscode/settings.json:
{
"C_Cpp.intelliSenseEngine": "Default"
}compile_commands.json:
[
{
"directory": "foo",
"file": "main.c",
"arguments":
[
"/usb/bin/clang",
"-Ibar",
"foo/main.c"
],
"command": "/usr/bin/clang -Ibar foo/main.c"
}
]foo/main.c:
#include "header.h"
int main(int argc, char** argv)
{
return RETURN_VALUE;
}bar/header.h:
#define RETURN_VALUE (0)build.sh:
#!/bin/bash
clang -Ibar foo/main.cReacted by Mika Fischer, Roman Shtylman, Oliver Freyermuth, juxeii, Nathan V. Morrical, Christian Venegas, poppolopoppo, Janne Kummer, Yuriy Ostapenko, David Dovodov and 3 more
Activity
sean-mcmanus commentedon Mar 21, 2018
Go to Definition doesn't use compile_commands.json data yet, which is only used to configure includePath and defines for IntelliSense -- only the browse.path affects Go to Definition, and it's recursive. I think we're using #255 to track that. Although it would be relatively simple to enable Go to Definition for definitions in the current translation unit (or declarations), enabling this for definitions in other translations is a major task.
Let us know if you're experiencing other bugs with compile_commands.json. Our pending 0.16.0 should fix some compile_commands.json issues, such as enabling forced includes and compiler defines to be picked up correctly.
sean-mcmanus commentedon Mar 21, 2018
Uh...but maybe we could use the include paths parsed from the compile_commands to populate the browse.path? The problem is that it wasn't designed to be used that way.
tksuoran commentedon Mar 22, 2018
My understanding is that both "includePath" and "browse" are inferior, because they are global. IMHO this is a design flaw. Global settings for includes and defines could perhaps work for a single "visual studio project", for example a library, but not a whole "visual studio solution" that contains several libraries, each with their own include paths and defines. And even within single project, sometimes there are file specific settings (includes, defines etc.).
compileCommands allows to set includes and defines individually for each file, and it allows to use VSCode similar to a Visual Studio solution which contains multiple projects.
Populating browse from compileCommands obviously would fail when different files have different settings. The settings should be file specific, and compileCommands does decent job with that.
sean-mcmanus commentedon Mar 22, 2018
The VS Code workspace and workspaceFolder correspond to the VS solution and project, so in that respect the includePath is settable per-folder/project, but not file-specific ones that compile commands enables. The other alternative method using one workspaceFolder is to switch configs when you change to subfolders with different includePaths, but that would be sort of annoying to do if you switching a lot.
The browse.path is for global symbols for the entire workspace folder. Only one database is created. Typically, you just set it to ${workspaceFolder} and then any external libraries that are not part of the workspace. browse.path is not used in compilation of a file like the includePath is.
We discussed having the includePath be a more complicated structure that enabled it to be set on a per directory/file basis, but decided it was an unnecessary complication and we haven't any user requests for that yet.
My idea for populating the browse.path based on the compileCommands would basically be to just add all the paths (assuming our code that removes duplicates works correctly). However, this may not be a good idea, and it might be better to force users to manually set it to the root paths they want symbols for.
And our goal is to replace most of the browse.path-based features with the "IntelliSense" implementation (which would use compile commands). We only have single translation unit IntelliSense implemented right now though, so external definitions require the browse.path symbol info.
bobbrow commentedon Mar 22, 2018
If browse.path is removed as part of the settings refactor (and recursive includes feature), then the extensionwould need to compute the equivalent of browse.path for the compile_commands.json case. I would want to do this task as part of that work to reduce the potential for confusion.
[-]c_cpp_properties.json / compileCommands / Go to Definition not working[/-][+]auto-populate the browse.path when compileCommands is used[/+]4 remaining items
ed-alertedh commentedon Jul 3, 2019
What I gather from other issues is that the reason for holding off is due to an eventual plan to consolidate all the paths into one setting, is that a fair summary?
I would really like this feature as I am trying to use conan to manage my dependencies and this means the locations of the headers are dynamic, e.g.
/home/devuser/.conan/data/zlib/1.2.11/conan/stable/package/880e5642afc26dae7f4019501d6632b3a6dba009/includeThis seems to be the only remaining thing stopping me from being able to configure everything with Conan and CMake without needing to manually keep
c_cpp_properties.jsonin sync. As it is, having the include paths automatically set up is a pretty nice start!sean-mcmanus commentedon Jul 3, 2019
The reason is that the work hasn't been high enough priority, but since we're fixing compile_command.json related issues for July/August, maybe we could fix this?
In most cases, we expect users to be able to set the browse.path to the root of all folders that will need to have global symbols found, as a workaround for us not automatically doing that based on the compile_commands.json.
sean-mcmanus commentedon Jul 15, 2019
I think the command is "-I", which we support ("bar" is the argument folder). The issue is we don't use the -I/etc. include paths to populate the browse.path...might be as simple as copying them over?
Jasdriel commentedon Jul 15, 2019
I am actually trying to replicate the problem but for me go to definition works well using only compile commands
sean-mcmanus commentedon Jul 15, 2019
The -I path has to be outside of the ${workspaceFolder} (I think that's used as the default browse.path).
Jasdriel commentedon Jul 15, 2019
I create an external folder with a header, and still working.
johschmitz commentedon Jul 16, 2019
When you have additional libraries that you need to link against installed in /opt/libname and you have set LD_LIBRARY_PATH and PKG_CONFIG_PATH to let cmake find them, does that work, does vscode find those header files now just by looking at the compile_commands.json?
sean-mcmanus commentedon Jul 16, 2019
Johannes Schmitz (@johschmitz) Our extension never deals with "linking". If the libraries have headers that are referenced in the compile_commands.json, then it will correctly be used for IntelliSense, but the files are only parsed after being opened (or from another opened file that includes it), so the symbols may not be found globally, i.e. Go to Def may fail (if they're not under the workspace folder). This fix for this issue would be to make sure all the paths referenced in the compile_commands.json get added to the browse.path.
johschmitz commentedon Jul 16, 2019
I am aware that the extension doesn't deal with linking, I just wanted to provide a complete scenario or maybe you call it "user story".
What I would expect is what you describe in your last sentence. All referenced paths should be added indeed.
bobbrow commentedon Mar 12, 2020
Duplicate of #1163