-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
"Other" commands line args (-mcpu, etc.) from compile_commands.json aren't used for system include/define querying #1755
Comments
Could you give us a timeline when this will get attention? I think this will be quite useful as this will clear most of the false positives in the Problems tab and would not require manual work to edit the vscode configuration files. Currently I am working on a large project that uses CMake. The thing that bugs me a bit is the integration with CMake. In the perfect case I would be able to load a CMake project which will then:
I not sure if this is the right issue to post into, but I think it is connected. Also my suggestion will probably include work in the main VSCode repository. And last, but not least, thank you for the great work and keep up! |
Your issues sounds different. If you're using CMake, you should already be able to set -DCMAKE_EXPORT_COMPILE_COMMANDS=1 and set the path in the compileCommands property in c_cpp_properties.json. This should work in most scenarios, but special cases may have issues that require the compilerPath to also be set, optionally with args. We already support the basic include path and defines args in the compileCommands, it's just the "other" ones we don't handle on a per-file basis and would have be globally set in the compilerPath until this is implemented. And there's a bug on Windows where non-cl.exe compilers require the compilerPath to be set if compileCommands is used. Also, the browse.path (root path for all workspace symbol parsing) is not set by compileCommands or compilerPath yet, but the browse.path is recursive and not order dependen, so it's easier to set. |
Thank you for the reply. I was talking about a better out of the box experience for CMake projects and reading compile_commands.json is a good step to it. But the problem is slightly different: repo root
Assuming CMake for all executables and libraries and the repo root is opened in VSCode. Edit: Are there any obstacles or complications for setting the browse path from compileCommands? This will allow loading of multiple projects each of with its own files with the multiple workspace concept? If no obstacles, I might try to implement it. |
I think this is necessary. In the args, there are some option, e.g. '-D' , '-I' sounds useful.
We seems needs:
|
@loaden, the extension does parse out the -D and -I options from the "command" when the IntelliSense engine starts. Sean was referring to the args that would have an effect on the system include path or defines when the compiler is queried for its default values. @Ju57iCe, we should be able to change the compileCommands property to accept an array of strings instead of just a single string. As a workaround in the meantime, you could create a "configuration" for each of your source folders and switch between them when working on the source files in the different projects. I have been considering adding a property to the configuration object that allows you to specify a glob pattern for the files that the configuration pertains to. This would allow the extension to auto-switch between configs depending on the active source file. |
Thanks a bunch for the suggestions. And populating the browse.path from the compile commands will be perfect. Keep up the good work guys! |
@sean-mcmanus I think this case is quite important. We are looking at using vs code more widely and one of the blockers is how much expert configuration it seems to need. We'd like people to just point it at the top of a git tree that uses cmake, or the linux kernel and have everything auto-setup based on that. cmake is the most complete extension right now, and it tells vs code all the right compiler data through compile_commands.json. For instance, if I have a simple project just uses "gcc -c .." in its compile_commands.json then ordinary C code that compiles fine like this: int main(const char *argv, int argc)
{
shmget(IPC_PRIVATE, 10, SHM_HUGETLB | SHM_R | SHM_W);
return 0;
} Gets problems in vs code because vs code has ignored compile_commands and configured its internal compiler for some other c std that sets STRICT_ANSI. The default behavior for gcc is -std=gnuXX. The desired behavior is for vs code to autoconfigure itself entirely based on the comple_commands with no required user intervenion. This includes intellisense and the browse engine. It is incredibly hard for users to go from 'why does SHM_HUGETLB get a bogus problem' to 'oh I need to change my C standard in VS code to gnu11'. There is no obvious linkage, and indeed new users will have no idea that the vs code C standard setting even exists or what it does. |
@jgunthorpe We recommend cmake users use the CMake Tools extension as their 1st choice for configuring, without compile_commands.json. It can properly set the c/cppStandard (although the gnu standards don't work yet). |
@sean-mcmanus ??? I am testing with the CMake Tools extenion, and it invokes cmake with options to force it to produce compile_command.json?? Are you saying it shouldn't do that? I'm confused how things can possibly work with out compile_command.json, or how the CMake Tools extension is supposed to automatically detect that the cmake files are switching into gnu11 mode. There isn't a cmake standard tool for that... |
@jgunthorpe CMake Tools implements our vscode-cpptools-api (https://github.com/microsoft/vscode-cpptools-api/blob/master/api.ts) and sends us "custom configuration" data with all the config info we need. The generation of a compile_commands.json is not needed, i.e. just set our configurationProvider (to "vector-of-bool.cmake-tools" or "ms-vscode.cmake-tools" and unset the compileCommands setting -- we had a bug that would prompt users to set the compileCommands even when the configurationProvider was set, but it's been fixed in our Insiders release. |
@sean-mcmanus Yes, I was certainly prompted, and if that is a bug, OK. Hum, I see, cmake-tools requests the compilation database and it looks like it only uses it with tryCompileFile - which seems like a lot of work when it could just invoke ninja directly to compile a single object file. I also see that cmake-tools contains a command line parser for gcc options and does parse the -std option (parseCStandard). Shouldn't this all be uniform? A single routine to convert a unix compiler command line into the internal representation? cmake-tools will autodetect the C std because it has that built into the command line parser, but compile_commands.json will not? But I suppose that answers my question in as far as cmake. When the gnu11 support is added to the language services parseCStandard and related can be updated and this will work automatically for cmake projects? Thanks |
I'm not sure how we'd make it more uniform (code lives at different locations). Yeah, when we add the gnu standards, it should work for cmake projects. |
Hi, As a 'me too', my use case is that I have a large monolithic project with much legacy code which has .c files some of which are compiled 'c++' others as 'c', also there are many files that have 'special' compilation configurations. Using the compile commands compiler flags would resolve the problems that are blocking me using VSC, for this project. |
This should be addressed in 0.29.0 (which is not released yet). |
Should be fixed with 0.29.0. |
This was intentionally delayed for later. If the args are needed the workaround is to set them in the compilerPath, but it's global for all files. Is this scenario important to anyone? We didn't get complaints about this in the 0.16.0 insider releases.
UPDATE: This is only for the special flags that modify compiler defaults, not the normal -D/I, e.g. args like -mcpu=cortex-m4 -mthumb -mfloat-abi=hard -mfpu=fpv4-sp-d16.
UPDATE: -std is sort of special case because it also sets flags we pass to our compiler -- not sure if it should overwrite the cppStandard or cStandard (which have a smaller range of values than -std).
The text was updated successfully, but these errors were encountered: