Go to Definition (F12) will list many redundant files

[gtianw]

Environment

• OS and Version: Windows_NT x64 10.0.19045
• VS Code Version: 1.100.2 (system setup)
• C/C++ Extension Version: 1.25.3
• If using SSH remote, specify OS of remote machine: WSL2, Ubuntu20.04

Bug Summary and Steps to Reproduce

Bug Summary: Go to Definition will list many redundant files. These files are not included by CMakeLists.txt or compile_commands.json. duplicates #11887, but it is no longer active.

Steps to reproduce:

1. Download code from https://github.com/gtianw/Issue/tree/cpptools/13660
2. F12 or Ctrl+LeftClick on target_prt.

Expected behavior: Only parse the file included by CMakeLists.txt or compile_commands.json, and directly jump to function definitions.

[gtianw]

In a real project, there may be dozens of targets.

[sean-mcmanus]

@gtianw It looks like this is caused by the parent folder of main.cpp being the workspaceFolder root, so both CMake Tools and compile commands are recursively parsing the workspaceFolder for symbols. I believe that is "by design" but it's possible we could implement some design change. You could potentially use

    "C_Cpp.files.exclude": {
        "**/target2": true,
        "**/target3": true
    }

@Colengms might have additional workaround suggestions.

[gtianw]

I will switch between multiple targets according to the usage scenario, and very frequently, so C_Cpp.files.exclude is not suitable.

[sean-mcmanus]

@gtianw Are you able to not have the parent folder be used, i.e. by not having any source files at the root?

[sean-mcmanus]

@gtianw Otherwise, did you have some sort of design change in mind (that doesn't break existing user scenarios) to prevent files at the rule from causing recursive tag parsing of all the subfolders?

[gtianw]

@gtianw Are you able to not have the parent folder be used, i.e. by not having any source files at the root?

I tried, but the problem persists.

[gtianw]

@gtianw Otherwise, did you have some sort of design change in mind (that doesn't break existing user scenarios) to prevent files at the rule from causing recursive tag parsing of all the subfolders?

Maybe add a new option in c_cpp_properties.json, similar to limitSymbolToIncludeHeaders. When this option is enabled, only included files are parsed. It would be better if there was a global option that each Configuration name inherited by default.

[sean-mcmanus]

@gtianw Without the root folder having a source file, are you able to tell from the logging why the entire workspace is being parsed recursively? If not, I'll try to investigate tomorrow. It could be either a bug with the cmake tools provider sending an incorrect browse path or a similar bug with how the browse path for compile_commands.json is computed (or it could be "by design" unless "includePath" or "browse.path" is overriden in the c_cpp_properties.json, I can't remember).

[gtianw]

I have moved main.cpp from the root directory to the src subdirectory. You can download the code from https://github.com/gtianw/Issue/tree/cpptools/13660 and reproduce the issue.

[sean-mcmanus]

@gtianw When main.cpp is moved to src with the CMake configuration (with CMake Tools) I do not repro the issue. The logging indicates that the workspace root is not being sent as a browse condiguration for CMake Tools. You may need to do a CMake: Delete Cache and Reconfigure or Reload Window or C/C++: Reset IntelliSense Database to get the configuration updated (not sure). I'll try with compileCommands...

[sean-mcmanus]

@gtianw Oops, sorry, I had a C_Cpp.files.exclude set. I reproed the bug still, investigating...

[sean-mcmanus]

@gtianw Setting/adding

            "browse": {
                "path": []
            }

in the configuration fixes it. I'm not sure if that is "by design" or not. I had includePath set to [] but that didn't work (as I had expected).

[sean-mcmanus]

@gtianw We decided to change the default behavior in the configurationProvider case to be what you expected and to not merge in the browse.path from the c_cpp_properties.json unless mergeConfigurations is set (https://github.com/microsoft/vscode-cpptools/pull/13678), i.e. when that change goes in, browse.path won't need to be explicitly set to [].

We're not sure yet about the compileCommands case yet (i.e. that may still require the browse.path to be set to match the behavior with the configurationProvider), which is somewhat being tracked at https://github.com/microsoft/vscode-cpptools/issues/8274 (although in that case it just mentions merging forcedIncludes, and it's asking to change it to merge instead of to not merge, although I think any change we make to this would probably unify the behavior to what configurationsProviders does).

[gtianw]

@gtianw Setting/adding

        "browse": {
            "path": []
        }

in the configuration fixes it. I'm not sure if that is "by design" or not. I had includePath set to [] but that didn't work (as I had expected).

@sean-mcmanus This is indeed a workaround, but it has significant limitations. Once I accidentally open redundant files, (i.e. target2/target.cpp when using target1 configuration), they will be added to the candidate.

[sean-mcmanus]

@gtianw If you open a file that is not normally tag parsed for symbols, we will tag parse it on open -- otherwise, document symbols won't work for the opened file -- but when you do a reload window after the file is closed, the symbols should be cleaned up after the workspace parsing phase is finished. We could potentially add a feature request to have a setting to not tag parse open files that are not normally parsed during workspace parsing.

[sean-mcmanus]

@gtianw Fixed for the configuration provider case with https://github.com/microsoft/vscode-cpptools/releases/tag/v1.26.2 .

[sean-mcmanus]

I'm closing this bug https://github.com/microsoft/vscode-cpptools/issues/8274 is still tracking handling the compile_commands.json case.

[gtianw]

We could potentially add a feature request to have a setting to not tag parse open files that are not normally parsed during workspace parsing.

@sean-mcmanus Which issue is used to track this new feature?

[sean-mcmanus]

@gtianw I don't think we've created one yet.