Skip to content
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

@response file failed to used in clang-tidy.exe with a large project(such as ue4.28) #8843

Closed
SambeauWang opened this issue Feb 11, 2022 · 27 comments
Assignees
Labels
Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. fixed Check the Milestone for the release in which the fix is or will be available. Language Service
Milestone

Comments

@SambeauWang
Copy link

Bug type: Language Service

Describe the bug

  • OS and Version: Windows 10 21H2
  • VS Code Version: 1.63.1
  • C/C++ Extension Version: 1.8.5

Steps to reproduce

  1. get ue4.28 source code, create default fps demo and build with vs2022
  2. use ue4 script to generate vscode workspace
  3. Open a cpp file and clang-tidy fail to search include path

Expected behavior
(1) a new choice for use -p <build-path> instead of -- for compile info in clang-tidy(https://clang.llvm.org/extra/clang-tidy/).
(2) use Clang-tidy feature in unreal engine vscode-cpptool

Code sample and logs

  • Configurations in c_cpp_properties.json
{
    "configurations": [
        {
            "name": "ue4_28Editor Editor Win64 Development (ue4_28)",
            "intelliSenseMode": "msvc-x64",
            "compileCommands": "F:\\UE4\\ue4_28\\.vscode\\compileCommands_ue4_28.json",
            "cStandard": "c17",
            "cppStandard": "c++17"
        },
        {
            "name": "Win32",
            "intelliSenseMode": "msvc-x64",
            "compileCommands": "F:\\UE4\\ue4_28\\.vscode\\compileCommands_Default.json",
            "cStandard": "c17",
            "cppStandard": "c++17"
        }
    ],
    "version": 4
}
  • Logs from running C/C++: Log Diagnostics from the VS Code command palette
cpptools/getCodeActions: G:\XXX\Source\XXX\Private\XXXEffect.cpp (id: 12)
cpptools/abortRequest
cpptools/getDocumentSymbols
"c:\Users\XXX\.vscode\extensions\ms-vscode.cpptools-1.8.5/bin/../LLVM/bin/clang-tidy.exe"
--header-filter="(G:/master/project|G:\\master\\project)"
--quiet
"G:/XXX/Source/XXX/Private/XXXEffect.cpp"
--
-std=c++17
-xc++
-Wno-pragma-pack
-includeG:\XXX\Source\Intermediate\Build\Win64\UE4Editor\Development\XXX\Definitions.XXX.h
-include"G:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Development\Engine\SharedPCH.Engine.h"
@"C:/Users/XXX/AppData/Local/Temp/cppADAC.tmp"
Error while processing G:\XXX\Source\XXX\Private\XXXEffect.cpp.
G:\XXX\Source\XXX\Private\XXXEffect.cpp:1:10: error: 'XXXComponent.h' file not found [clang-diagnostic-error]
#include "XXXComponent.h"
         ^~~~~~~~~~~~~~~~~~~~~~~
  Processing folder (recursive): G:/XXX/SOURCE/
  Processing folder (recursive):

Additional context
I try to use clang-tidy.exe in cmd, and try to use @response file. It failed to use @response file in clang-tidy.
(1) I try to use "C:\Users\XXX\.vscode\extensions\ms-vscode.cpptools-1.8.5\LLVM\bin\clang-tidy.exe" --quiet XXX.cpp -- @G:\master\compile_flags.txt. The result is error: 'XXXComponent.h' file not found
@response file is follewed.

...
-IG:\master\ue4_engine\Engine\Source\Runtime\AudioMixerCore 
-IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\PropertyAccess 
-IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess\Public 
-IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess 
-IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\UnrealEd 
...

(2) I try to move -- to @response file and merge all args as one line(the command is "C:\Users\XXX\.vscode\extensions\ms-vscode.cpptools-1.8.5\LLVM\bin\clang-tidy.exe" --quiet XXX.cpp @G:\master\compile_flags.txt). error: 'XXXComponent.h' file not found is not exit.

@response file is follewed.

-- ... -IG:\master\ue4_engine\Engine\Source\Runtime\AudioMixerCore -IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\PropertyAccess -IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess\Public -IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess -IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\UnrealEd ...

(3) I try to use -p XXX.txt instead of --. Such as "C:\Users\XXX\.vscode\extensions\ms-vscode.cpptools-1.8.5\LLVM\bin\clang-tidy.exe" --quiet XXX.cpp -p G:\master\compile_flags.txt. It work fine with ue4.28 project.

compile_flags.txt is follewed.

...
-IG:\master\ue4_engine\Engine\Source\Runtime\AudioMixerCore 
-IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\PropertyAccess 
-IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess\Public 
-IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess 
-IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\UnrealEd 
...

Then I try to use -p XXX.txt to cpptools extension as args, it fail working. -- and -p XXX.txt may be conflict(https://clang.llvm.org/extra/clang-tidy/).

@sean-mcmanus sean-mcmanus self-assigned this Feb 11, 2022
@sean-mcmanus sean-mcmanus added bug Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. Language Service labels Feb 11, 2022
@sean-mcmanus sean-mcmanus added this to the 1.9 milestone Feb 11, 2022
@sean-mcmanus sean-mcmanus removed the bug label Feb 11, 2022
@sean-mcmanus sean-mcmanus removed this from the 1.9 milestone Feb 11, 2022
@sean-mcmanus
Copy link
Collaborator

Our code analysis implementation doesn't directly process the compile commands file or the response files contained in it -- we rely on the IntelliSense configuration. You should not be trying to pass in any additional "-p or --" as args. Is IntelliSense able to find the headers when opening XXXEffect.cpp? If so, then code analysis should also be able to without any additional configuration.

@sean-mcmanus sean-mcmanus added more info needed The issue report is not actionable in its current state not reproing We're not able to reproduce the issue (it's unlikely to get fixed until we find one). labels Feb 11, 2022
@sean-mcmanus
Copy link
Collaborator

sean-mcmanus commented Feb 11, 2022

FYI, the compile commands generated by Unreal Engine may be incorrect, leading to incorrect IntelliSense and therefore incorrect code analysis configuration (another user hit this in issue #8812 ).

@sean-mcmanus
Copy link
Collaborator

Our extension will create a response file when there are too many include paths, which may occur with the Unreal Engine. Are you saying our extension has a bug in that case or is the compile commands itself incorrect?

@SambeauWang
Copy link
Author

For the log, extension will create a response file with -- as args when there are too many include paths. There maybe have a some problem which use -- as clang-tidy compile args will result in @response file failture . So I try to use clang-tidy.exe in cmd to test the response file. The command is "C:\Users\XXX\.vscode\extensions\ms-vscode.cpptools-1.8.5\LLVM\bin\clang-tidy.exe" --quiet XXX.cpp @G:\master\compile_flags.txt.

cpptools/getCodeActions: G:\XXX\Source\XXX\Private\XXXEffect.cpp (id: 12)
cpptools/abortRequest
cpptools/getDocumentSymbols
"c:\Users\XXX\.vscode\extensions\ms-vscode.cpptools-1.8.5/bin/../LLVM/bin/clang-tidy.exe"
--header-filter="(G:/master/project|G:\\master\\project)"
--quiet
"G:/XXX/Source/XXX/Private/XXXEffect.cpp"
--
-std=c++17
-xc++
-Wno-pragma-pack
-includeG:\XXX\Source\Intermediate\Build\Win64\UE4Editor\Development\XXX\Definitions.XXX.h
-include"G:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Development\Engine\SharedPCH.Engine.h"
@"C:/Users/XXX/AppData/Local/Temp/cppADAC.tmp"
Error while processing G:\XXX\Source\XXX\Private\XXXEffect.cpp.
G:\XXX\Source\XXX\Private\XXXEffect.cpp:1:10: error: 'XXXComponent.h' file not found [clang-diagnostic-error]
#include "XXXComponent.h"
         ^~~~~~~~~~~~~~~~~~~~~~~
  Processing folder (recursive): G:/XXX/SOURCE/
  Processing folder (recursive):

response file is follewed.

...
-IG:\master\ue4_engine\Engine\Source\Runtime\AudioMixerCore 
-IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\PropertyAccess 
-IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess\Public 
-IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess 
-IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\UnrealEd 
...

Then, I try to use -p as clang-tidy compile args with same heards info, It work fine with my project.

@sean-mcmanus
Copy link
Collaborator

@SambeauWang Are you able to determine why the XXXComponent.h header can't be found, i.e. what folder is it located in and why that folder is not being added as a -I and/or why it's able to be found with -p? Is IntelliSense working with XXXEffect.cpp?

@SambeauWang
Copy link
Author

@sean-mcmanus I try to use the path of XXXComponent.h in cmd, such as C:\Users\XXX\.vscode\extensions\ms-vscode.cpptools-1.8.5\LLVM\bin\clang-tidy.exe" --quiet XXX.cpp -- -IG:/XXX/Source/XXX/Public. error: 'XXXComponent.h' file not found is not exit.

Then I try to move -- to @response file and merge all args as one line(the command is "C:\Users\XXX\.vscode\extensions\ms-vscode.cpptools-1.8.5\LLVM\bin\clang-tidy.exe" --quiet XXX.cpp @G:\master\compile_flags.txt). error: 'XXXComponent.h' file not found is not exit.
response file is follewed.

-- ... -IG:\master\ue4_engine\Engine\Source\Runtime\AudioMixerCore -IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\PropertyAccess -IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess\Public -IG:\master\ue4_engine\Engine\Source\Runtime\PropertyAccess -IG:\master\ue4_engine\Engine\Intermediate\Build\Win64\UE4Editor\Inc\UnrealEd ...

I think it maybe result in the error which the args is too long in response file for clang-tidy.exe.

@sean-mcmanus sean-mcmanus added the To Verify - Internal To verify if issue reproduces label Feb 15, 2022
@Colengms
Copy link
Collaborator

Hi @SambeauWang . Could this be a duplicate of #8812 ? I appear to be some issues with Unreal Editor currently generating incorrect compile_commands files.

@sean-mcmanus
Copy link
Collaborator

Can you run C/C++: Log Diagnostics after opening XXXEffect.cpp and see whether or not the path to XXXComponent.h is in the Includes list? That should be identical to what we are using in the -I in the response file we generate for clang-tidy.

@Colengms
Copy link
Collaborator

Hi @SambeauWang.

(1) a new choice for use -p instead of -- for compile info in clang-tidy

We should be passing all of the same arguments as were present in compile_commands.json (including response file contents), directly to clang-tidy using --. We support using clang-tidy in scenarios that do not involve compile-commands and are leveraging the results of that process instead of using the existing compile-commands.json file in your scenario. The same information is used to configure IntelliSense. If clang-tidy is failing to find a header file that would suggest either that same header is missing from IntelliSense, or there is perhaps a bug in which something is wrong or omitted when invoking clang-tidy.

For the source file involved, could you provide the complete build command line? (The compile-commands.json entry for that file, and the contents of the response file it uses.) If you could also provide the Log Diagnostics output Sean requested, we should be able to compare the 2 sets of information to see if there is anything inconsistent between them.

As I mentioned in my previous comment, there are known issues with UE 4.27 that result in headers not being found. I'm not sure if that has been addressed in 4.28. Where can I find 4.28? :)

@SambeauWang
Copy link
Author

@Colengms Thank you for help. (1) From the log, I think this extension use compile-commands.json(including response file contents) to create a temp response file which has a lot of -Ipath for clang-tidy.exe. And This extension use a command, such as clang.tidy.exe --quiet XXX.cpp -- @temp response file. So I try to use same command in cmd, And the result is show above.
(2) There maybe some issues with Unreal Editor currently generating incorrect compile_commands files. So I try to fix it as attachment(change suffix to cs)
VSCodeProjectFileGenerator.Log
.
(3) UE 4.28 is not release, so I pull master branch.
(4) The log info and rsp file is follewed.
LogDiagnostics.txt
XXXPhysicsRuntime.7.txt

@sean-mcmanus
Copy link
Collaborator

The Log Diagnostics file is actually logging from the C/C++ pane -- can you run the C/C++: Log Diagnostics command and get logging from the C++ Diagnostics pane?

@SambeauWang
Copy link
Author

The log from the C++ Diagnostics is follewed.

C++Diagnostics.txt

@sean-mcmanus
Copy link
Collaborator

It says "No active translation units." -- we need the C/C++: Log Diagnostics command to be run after the repro file (XXXEffect.cpp) is opened and IntelliSense has finished initializing (flame icon goes away).

@SambeauWang
Copy link
Author

"No active translation units." is still. flame icon goes away, and error is show in PROBLEMS panel
C++Diagnostics2.txt
.

@SambeauWang
Copy link
Author

image

@sean-mcmanus
Copy link
Collaborator

What is the error in the problems window? Does it say it can't find the header XXXComponent.h? Can you make sure C_Cpp.intelliSenseEngineFallback is set to "Disabled"?

@SambeauWang
Copy link
Author

image
image

@sean-mcmanus
Copy link
Collaborator

Those are clang diagnostic errors -- are you getting any IntelliSense errors (they appear as C++(<errorCodeNumber>)). Something is causing the IntelliSense process to not startup correctly. Does your "C/C++" logging (not the "diagnostics" one) show any "crash" messages? Your C_Cpp.intelliSenseEngine is set to "Default", right?

@SambeauWang
Copy link
Author

C_Cpp.intelliSenseEngine is set to "Tag parse". "C/C++" logging(LogDiagnostics.txt) is showed above.

@SambeauWang
Copy link
Author

After C_Cpp.intelliSenseEngine is set to "Default", the log of C/C++: Log Diagnostics :
C++Diagnostics3.txt

@v-ericawu v-ericawu added Verifying - Internal Verifying if issue reproduces and removed To Verify - Internal To verify if issue reproduces labels Feb 16, 2022
@sean-mcmanus
Copy link
Collaborator

sean-mcmanus commented Feb 16, 2022

So does IntelliSense give any error squiggles, particularly about missing headers? I compared the include paths and it appears that the IntelliSense has about 44 fewer include paths compared to the response file, but it seems like they may have been generated using different source files. If you're still getting missing headers reported during code analysis, can you provide the path to that header so I can see if it's in the IntelliSense includes (or you could check)?

Our code analysis feature wasn't designed/implemented to work without the IntelliSense engine...we could potentially add a feature request to support that and/or add a better warning message and/or disable it instead of having it run incorrectly.

The 2 GB memory usage of the IntelliSense engine suggests that it may be out of scope for our current implementation to handle well -- you may want to try switching to VS 2022 which has some additional optimizations that make Unreal Engine work better with less memory (which we don't have yet, see #3628): see https://devblogs.microsoft.com/cppblog/18x-faster-intellisense-for-unreal-engine-projects-in-visual-studio-2022/ .

@SambeauWang
Copy link
Author

SambeauWang commented Feb 17, 2022

@sean-mcmanus Thank you for help. Now I use VS2022 to work with Unreal Engine. But VSCode and cpptool extension is more lightweight and friendly. So I try to set C_Cpp.intelliSenseEngine is set to "Tag parse", and want to use clang-tidy.exe to do a simple grammar check.

It maybe result in it seems like they may have been generated using different source files which I replace some name with XXX. In error: 'Containers/ContainersFwd.h' file not found, ``Containers/ContainersFwd.h is location in "G:\master\ue4_engine\Engine\Source\Runtime\Core\Public". And there is no errors in IntelliSense. The path of 'Containers/ContainersFwd.h' header file is not missing for `XXXPhysicsRuntime.7.rsp` and IntelliSense(`C++Diagnostics3.txt`), and I think that it is more possibility that `clang-tidy` is conflict with `@response file` or args is too long for `clang-tidy.exe`. Because I try to use same command in cmd from `LogDiagnostics.txt`, the result show above.

image

@sean-mcmanus
Copy link
Collaborator

FYI, VS also has some clang-tidy support, but it's somewhat different: https://docs.microsoft.com/en-us/cpp/code-quality/clang-tidy?view=msvc-170 .

I've filed a feature request on enabling clang-tidy without IntelliSense at #8874 .

The G:\master\ue4_engine\Engine\Source\Runtime\Core\Public was not listed in the Includes for IntelliSense. If that was supposed to be then there could be some bug in our processing of the compile_commands.json/rsp file.

@github-actions
Copy link

This issue has been closed automatically because it needs more information and has not had recent activity.

@sean-mcmanus
Copy link
Collaborator

FYI, we found a bug with our response file handling (#9102) -- it should be fixed with our next 1.10.1 insiders. Multiple threads would try to use the same response file, so a workaround would be to set "C_Cpp.codeAnalysis.maxConcurrentThreads": 1

@sean-mcmanus
Copy link
Collaborator

Yeah, I'm seeing @response files not working with clang-tidy -- seems like our tests were inheriting the include paths from the environment instead of getting them from the response file :(

@sean-mcmanus sean-mcmanus removed more info needed The issue report is not actionable in its current state not reproing We're not able to reproduce the issue (it's unlikely to get fixed until we find one). Verifying - Internal Verifying if issue reproduces labels May 6, 2022
@sean-mcmanus sean-mcmanus added this to the 1.10.1 milestone May 6, 2022
@sean-mcmanus sean-mcmanus reopened this May 6, 2022
@sean-mcmanus sean-mcmanus added the fixed Check the Milestone for the release in which the fix is or will be available. label May 6, 2022
@sean-mcmanus
Copy link
Collaborator

This should be fixed with 1.10.2 (pre-release).

@github-actions github-actions bot locked and limited conversation to collaborators Jul 24, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. fixed Check the Milestone for the release in which the fix is or will be available. Language Service
Projects
None yet
Development

No branches or pull requests

4 participants