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

Switch to using -p for clang-tidy when configured by compileCommands #8740

Closed
devzeb opened this issue Jan 29, 2022 · 12 comments
Closed

Switch to using -p for clang-tidy when configured by compileCommands #8740

devzeb opened this issue Jan 29, 2022 · 12 comments
Assignees
Labels
bug 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 quick fix
Milestone

Comments

@devzeb
Copy link

devzeb commented Jan 29, 2022

Bug type: Language Service

Describe the bug

  • OS and Version: Windows 10 Pro 20H2
  • VS Code Version: 1.63.2
  • C/C++ Extension Version: v1.8.1 Prerelease

In my project I'm using the cmsis_compiler.h header which checks for the compiler used.

A minimal example of the problematic header looks like this:

//error_header.h
#ifndef TEST_HEADER_
#define TEST_HEADER_

#if defined ( __GNUC__ )
  //include some stuff
#else
  #error not compiling with gcc
#endif

#endif

I'm compiling with the GNU arm-none-eabi toolchain, so __GNUC__ is defined.

When I include this header in a source file, some warnings, which were previously shown in the editor, are now not shown anymore. See the following two pictures:

//test.cpp
grafik
grafik

However, if i manually invoke clang-tidy from the command line (same arguments as set in setting.json), all warnings are shown like in the first picture:

clang-tidy -p D:\code\display\build d:\code\display\common\utilities\tools\src\test.cpp <

6 warnings generated.
D:/code/display/common/utilities/tools/src/test.cpp:4:9: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
    int a;
        ^
          = 0
D:/code/display/common/utilities/tools/src/test.cpp:5:9: warning: Value stored to 'b' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
    int b = a + 2;
        ^   ~~~~~
D:/code/display/common/utilities/tools/src/test.cpp:5:9: note: Value stored to 'b' during its initialization is never read
    int b = a + 2;
        ^   ~~~~~
D:/code/display/common/utilities/tools/src/test.cpp:5:9: warning: unused variable 'b' [clang-diagnostic-unused-variable]
    int b = a + 2;
        ^
D:/code/display/common/utilities/tools/src/test.cpp:5:13: warning: variable 'a' is uninitialized when used here [clang-diagnostic-uninitialized]
    int b = a + 2;
            ^
D:/code/display/common/utilities/tools/src/test.cpp:4:10: note: initialize the variable 'a' to silence this warning
    int a;
         ^
          = 0
D:/code/display/common/utilities/tools/src/test.cpp:5:15: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
    int b = a + 2;
            ~ ^
D:/code/display/common/utilities/tools/src/test.cpp:4:5: note: 'a' declared without an initial value
    int a;
    ^~~~~
D:/code/display/common/utilities/tools/src/test.cpp:5:15: note: The left operand of '+' is a garbage value
    int b = a + 2;
            ~ ^
Suppressed 1 warnings (1 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

Relevant section of settings.json:

    "C_Cpp.codeAnalysis.clangTidy.args": [
        "-p","${workspaceFolder}/build",
    ]   

The build folder contains a compile_commands.json which, to my understanding, is used in either case (command line and by vscode).

It has to do something with the conditional #error directive.
If my problematic header just contains #error without a condition, the command line invocation of clang-tidy does also not display all warnings.

It seems to me that vscode is invoking clang-tidy differently than my command.
Is there any way to see what exact command vscode is calling to invoke clang tidy?

@devzeb
Copy link
Author

devzeb commented Jan 29, 2022

This problem also happens when I include standard headers.
See the following two pictures:
grafik
grafik

@devzeb devzeb changed the title clang-tidy: editor does not show all warnings when included header contains #error directive, but command line invokation does clang-tidy: editor does not show all warnings, but command line invokation does Jan 29, 2022
@devzeb devzeb changed the title clang-tidy: editor does not show all warnings, but command line invokation does clang-tidy: editor does not show all warnings, but command line invocation does Jan 29, 2022
@devzeb
Copy link
Author

devzeb commented Jan 29, 2022

Edit: It's still not solved by this workaround, see next comment.


I figured this out.
clang-tidy is using its own standard include directories.
This makes clang-tidy run with "clangs configuration" instead of the configuration of the crosscompiler "arm-none-eabi-gcc"

In order to fix this, you need to pass -nostdinc and -nostdinc++ to clang-tidy as extra arguments (either through --extra-arg or by making them appear in compile_commands.json).
If you do this, clang will not be able to find files from the standard libraries, so it's REQUIRED to specify your compilers default include directories.

These can be found out by using the following command:
echo "#include <bogus.h>" | <path-to-your-crosscompiler-here> -v -x c++ - ( for c++ project, credits to
Christian Ternus in his [answer]https://stackoverflow.com/q/19580758) )
look for "#include <...> search starts here:" in the commands output and add the following include directories to the invocation of clang-tidy (again trough --extra-arg or by making them appear in compile_commands.json).

Still, it's unclear to me why the command line command worked in the first place.

I don't consider this fixed.
Vscode should behave no differently than using clang-tidy from the command line.

@devzeb
Copy link
Author

devzeb commented Jan 30, 2022

Even with the workaround of the above comment applied, the behaviour is still very inconsistent.

Some standard headers still seem to break the warnings in editor:
grafik
grafik

The command line still shows the expected output, vscode doesn't

Spoiler: Command output
15853 warnings generated.
warning: argument unused during compilation: '-nostdinc++' [clang-diagnostic-unused-command-line-argument]
D:/code/display/project/shared/gfx/database/screens/src/test.cpp:5:9: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
    int a;
        ^
          = 0
D:/code/display/project/shared/gfx/database/screens/src/test.cpp:6:9: warning: Value stored to 'b' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
    int b = a+2;
        ^   ~~~
D:/code/display/project/shared/gfx/database/screens/src/test.cpp:6:9: note: Value stored to 'b' during its initialization is never read
    int b = a+2;
        ^   ~~~
D:/code/display/project/shared/gfx/database/screens/src/test.cpp:6:9: warning: unused variable 'b' [clang-diagnostic-unused-variable]
    int b = a+2;
        ^
D:/code/display/project/shared/gfx/database/screens/src/test.cpp:6:13: warning: variable 'a' is uninitialized when used here [clang-diagnostic-uninitialized]
    int b = a+2;
            ^
D:/code/display/project/shared/gfx/database/screens/src/test.cpp:5:10: note: initialize the variable 'a' to silence this warning
    int a;
         ^
          = 0
D:/code/display/project/shared/gfx/database/screens/src/test.cpp:6:14: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult] 
    int b = a+2;
            ~^
D:/code/display/project/shared/gfx/database/screens/src/test.cpp:5:5: note: 'a' declared without an initial value
    int a;
    ^~~~~
D:/code/display/project/shared/gfx/database/screens/src/test.cpp:6:14: note: The left operand of '+' is a garbage value
    int b = a+2;
            ~^
Suppressed 15847 warnings (15847 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

@sean-mcmanus sean-mcmanus self-assigned this Jan 31, 2022
@sean-mcmanus
Copy link
Collaborator

You can see the command line we run via looking at the logging with C_Cpp.loggingLevel set to "Debug".

@sean-mcmanus sean-mcmanus added this to the 1.9.0 (insiders) milestone Jan 31, 2022
@sean-mcmanus sean-mcmanus added bug Feature: Code Analysis Related to integration with clang-tidy, cppcheck, cl.exe /analyze, etc. Language Service labels Jan 31, 2022
@sean-mcmanus sean-mcmanus added the fixed Check the Milestone for the release in which the fix is or will be available. label Jan 31, 2022
@sean-mcmanus
Copy link
Collaborator

sean-mcmanus commented Jan 31, 2022

We were incorrectly removing a bunch of system defines, including __GNUC__.

The fix should be in our next Insiders later this week.

UPDATE: The insiders got delayed.

@michelleangela
Copy link
Contributor

The fix for this issue is now available in version Insiders 1.9.0.

@devzeb
Copy link
Author

devzeb commented Feb 25, 2022

I just tested it with 1.9.1, still got the same issue when working with standard template library headers.

See the following pictures:
grafik
grafik

The problem with the minimal header of my first comment (error_header.h) is now fixed though.

Command line output (working as expected):

PS D:\code\platform> clang-tidy -p .\build-Debug-arm-none-eabi-toolchain\ project\application\app\src\test.cpp
8739 warnings generated.
D:/code/platform/project/application/app/src/test.cpp:5:9: warning: variable 'a' is not initialized [cppcoreguidelines-init-variables]
    int a;
        ^
          = 0
D:/code/platform/project/application/app/src/test.cpp:6:9: warning: Value stored to 'b' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
    int b = a + 2;
        ^   ~~~~~
D:/code/platform/project/application/app/src/test.cpp:6:9: note: Value stored to 'b' during its initialization is never read
    int b = a + 2;
        ^   ~~~~~
D:/code/platform/project/application/app/src/test.cpp:6:9: warning: unused variable 'b' [clang-diagnostic-unused-variable]
    int b = a + 2;
        ^
D:/code/platform/project/application/app/src/test.cpp:6:13: warning: variable 'a' is uninitialized when used here [clang-diagnostic-uninitialized]
    int b = a + 2;
            ^
D:/code/platform/project/application/app/src/test.cpp:5:10: note: initialize the variable 'a' to silence this warning
    int a;
         ^
          = 0
D:/code/platform/project/application/app/src/test.cpp:6:15: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
    int b = a + 2;
            ~ ^
D:/code/platform/project/application/app/src/test.cpp:5:5: note: 'a' declared without an initial value
    int a;
    ^~~~~
D:/code/platform/project/application/app/src/test.cpp:6:15: note: The left operand of '+' is a garbage value
    int b = a + 2;
            ~ ^
Suppressed 8734 warnings (8734 in non-user code).
Use -header-filter=.* to display errors from all non-system headers. Use -system-headers to display errors from system headers as well.

Also see the output of the C++ vscode extension when running code analysis:
clang-tidy-bug.txt

@devzeb
Copy link
Author

devzeb commented Feb 25, 2022

I tried to find out more information about this problem and came to the following conclusion:
If i run the exact same command as stated in the clang-tidy-bug.txt, but without the following lines, clang tidy does not crash:

-I"C:/Program Files (x86)/GNU Arm Embedded Toolchain/10 2021.10/arm-none-eabi/include/c++/10.3.1/arm-none-eabi"
-I"C:/Program Files (x86)/GNU Arm Embedded Toolchain/10 2021.10/arm-none-eabi/include/c++/10.3.1"
-I"C:/Program Files (x86)/GNU Arm Embedded Toolchain/10 2021.10/arm-none-eabi/include/c++/10.3.1/arm-none-eabi/thumb/v7e-m+fp/hard"
-I"C:/Program Files (x86)/GNU Arm Embedded Toolchain/10 2021.10/arm-none-eabi/include/c++/10.3.1/backward"
-I"C:/Program Files (x86)/GNU Arm Embedded Toolchain/10 2021.10/lib/gcc/arm-none-eabi/10.3.1/include"
-I"C:/Program Files (x86)/GNU Arm Embedded Toolchain/10 2021.10/lib/gcc/arm-none-eabi/10.3.1/include-fixed"
-I"C:/Program Files (x86)/GNU Arm Embedded Toolchain/10 2021.10/arm-none-eabi/include"

Why are these many defines and includes appended to the command line invocation of clang-tidy anyway?
Isn't this info already included in my compile_commands.json file?
Why does vscode not use the compile_commands.json?

As you can see in my previous comments, running clang-tidy with only the -p option and without any additional includes / defines, the output is correct.
Therefore, if vscode would run clang tidy like this, the code squiggles would be correct.

Can't you just invoke clang-tidy like that?

@sean-mcmanus sean-mcmanus removed the fixed Check the Milestone for the release in which the fix is or will be available. label Feb 25, 2022
@sean-mcmanus sean-mcmanus modified the milestones: 1.9.0 (insiders), 1.9 Feb 25, 2022
@sean-mcmanus
Copy link
Collaborator

I recall it was simpler to just query IntelliSense for its info (which it already parsed from compile commands) regardless of whether it was configured by a compile commands or configuration provider or c_cpp_properties.json instead of trying to get the compile_commands directly, which would require more work and we weren't aware of any reason to do that additional work.

I can look into switching to using -p in the compile commands scenario.

@sean-mcmanus sean-mcmanus changed the title clang-tidy: editor does not show all warnings, but command line invocation does Switch to using -p for clang-tidy when configured by compileCommands Feb 25, 2022
@sean-mcmanus sean-mcmanus modified the milestones: 1.9, 1.10 Feb 26, 2022
@devzeb
Copy link
Author

devzeb commented Feb 26, 2022

Thank you so much, my team is really looking forward to using clang-tidy, so your work on this is highly appreciated.
In my opinion, having the option to select either IntelliSense or compile_commands.json would make sense.

Please also notice that currently vscode is not correctly replacing the command variable ( ${command:cmake.buildDirectory} ) in the clang-tidy invocation (clang-tidy-bug.txt, line 5).

@sean-mcmanus
Copy link
Collaborator

I filed an issue about the command resolving at #8946.

@devzeb
Copy link
Author

devzeb commented Apr 25, 2022

I have just tested the new options in the v1.10.0 pre-release and am happy to report that clang-tidy is now working properly in our codebase.
Thank you very much for fixing this issue.

The only issue we are still facing at the moment is that for the C_Cpp.default.compileCommands option, Variables Reference are not resolved correctly.
Therefore, our build path needs to be hardcoded instead of specifying ${command:cmake.buildDirectory}.
But that issue is more related to ticket #8946, so I'm closing this issue for now.

Thanks again for your effort.

@devzeb devzeb closed this as completed Apr 25, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 9, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug 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 quick fix
Projects
None yet
Development

No branches or pull requests

3 participants