vscode-cmake-tools icon indicating copy to clipboard operation
vscode-cmake-tools copied to clipboard

Unable to set a header path as -isystem in clang-tidy?

Open arghness opened this issue 2 years ago • 11 comments
trafficstars

Environment

  • OS and Version: Ubuntu 22.04
  • VS Code Version: 1.80.1
  • C/C++ Extension Version: 1.16.3
  • If using SSH remote, specify OS of remote machine: Rocky Linux 8

Bug Summary and Steps to Reproduce

Bug Summary:

I am using the CMake extension and a custom build of Boost installed to a separate directory under /opt. CMake is building a compile_commands.json that adds it as -isystem . clang-tidy is adding it as -I , which is triggering various errors that I don't care about.

I've tried setting compileCommands "C_Cpp.default.compileCommands" and "C_Cpp.codeAnalysis.clangTidy.useBuildPath": true but then I get other basic include errors (e.g. "'stddef.h' file not found"), so I'm not sure if this fixes my issue or not.

It looks like "C_Cpp.codeAnalysis.clangTidy.args" may only apply to arguments before the filename being tested, so I haven't been able to see if I can override the -I with a -isystem.

Configuration and Logs

C_Cpp settings (taken from workspace file, as I do not have a c_cpp_properties.json):

"C_Cpp.clang_format_style": "file:${workspaceFolder}/XXXX/vscode/.clang-format",
"C_Cpp.codeAnalysis.clangTidy.args": [
	"--config-file=${workspaceFolder}/XXXX/vscode/.clang-tidy"
],
"C_Cpp.codeAnalysis.exclude": {
	// Various paths of 3rd party generated code.
},
"C_Cpp.default.configurationProvider": "ms-vscode.cmake-tools",
"C_Cpp.vcpkg.enabled": false,


-------- Diagnostics - 7/17/2023, 1:29:49 PM
Version: 1.16.3
Current Configuration:
{
    "name": "Linux",
    "includePath": [
        "XXXX/master/**"
    ],
    "defines": [],
    "compilerPath": "/usr/bin/gcc",
    "cStandard": "c17",
    "cppStandard": "gnu++14",
    "intelliSenseMode": "linux-gcc-x64",
    "compilerPathInCppPropertiesJson": "/usr/bin/gcc",
    "intelliSenseModeIsExplicit": false,
    "cStandardIsExplicit": false,
    "cppStandardIsExplicit": false,
    "mergeConfigurations": false,
    "compilerPathIsExplicit": false,
    "configurationProvider": "ms-vscode.cmake-tools",
    "browse": {
        "path": [
            "${workspaceFolder}/**"
        ],
        "limitSymbolsToIncludedHeaders": true
    }
}
Custom browse configuration: 
{
    "browsePath": [
        "XXXX/obj/CMakeFiles",
        ....
        "/opt/XXXX/XXXX-boost/include",
        ....
    ],
    "compilerPath": "/opt/rh/gcc-toolset-11/root/usr/bin/c++",
    "compilerArgs": [],
    "compilerFragments": [
        "-g",
        "-Wall",
        "-Wextra",
        "-Werror",
        "-Wno-deprecated-copy",
        "-std=gnu++17"
    ]
}
Custom configurations:
[ XXXX/core/tests/layer_decoder.cpp ]
{
    "includePath": [
        "XXXX/include",
        .... 
        "/opt/XXXX/XXXX-boost/include"
    ],
    "defines": [
        "BOOST_THREAD_DYN_LINK",
        "BOOST_THREAD_NO_LIB",
        "BOOST_UNIT_TEST_FRAMEWORK_DYN_LINK",
        "BOOST_UNIT_TEST_FRAMEWORK_NO_LIB",
        "FMT_SHARED"
    ],
    "compilerPath": "/opt/rh/gcc-toolset-11/root/usr/bin/c++",
    "compilerArgs": [],
    "compilerFragments": [
        "-g",
        "-Wall",
        "-Wextra",
        "-Werror",
        "-Wno-deprecated-copy",
        "-pthread",
        "-std=gnu++17"
    ]
}
cpptools version (native): 1.16.3.0
Translation Unit Mappings:
[ XXXX/core/tests/layer_decoder.cpp ]:
    XXXX/core/tests/layer_decoder.cpp
Translation Unit Configurations:
[ XXXX/core/tests/layer_decoder.cpp ]:
    Process ID: 3579914
    Memory Usage: 684 MB
    Compiler Path: /opt/rh/gcc-toolset-11/root/usr/bin/c++
    Includes:
        ....
        /opt/XXXX/XXXX-boost/include
        ....
        /opt/rh/gcc-toolset-11/root/usr/include/c++/11
        /opt/rh/gcc-toolset-11/root/usr/include/c++/11/x86_64-redhat-linux
        /opt/rh/gcc-toolset-11/root/usr/include/c++/11/backward
        /opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/include
        /usr/local/include
        /opt/rh/gcc-toolset-11/root/usr/include
        /usr/include
    Defines:
        BOOST_THREAD_DYN_LINK
        BOOST_THREAD_NO_LIB
        BOOST_UNIT_TEST_FRAMEWORK_DYN_LINK
        BOOST_UNIT_TEST_FRAMEWORK_NO_LIB
        FMT_SHARED
    Standard Version: c++17
    IntelliSense Mode: linux-gcc-x64
    Other Flags:
        --g++
        --gnu_version=110201
Total Memory Usage: 684 MB

------- Workspace parsing diagnostics -------
Number of files discovered (not excluded): 24486
Number of files parsed: 9734

Other Extensions

ms-vscode.cmake-tools

Additional context

No response

arghness avatar Jul 17 '23 14:07 arghness

I think this is a CMake Tools bug, because CMake Tools should send the -isystem include paths in compilerFragments...although I'm not sure if that could cause problems with ordering with the includePath includes, in which case, if -isystem includes exist then maybe includePath should not be used and all includes passed via arguments in compilerFragments (or compilerArgs).

sean-mcmanus avatar Jul 17 '23 17:07 sean-mcmanus

@arghness Could you please provide a repro sample that we could use to investigate this issue?

Additionally, where are the system header paths that are appearing in compile_commands and showing errors coming from? It'd be good to know where they are coming from, as it may be possible that CMake isn't aware of them because they are coming from a downstream tool.

Thanks!

gcampbell-msft avatar Jul 21 '23 19:07 gcampbell-msft

@gcampbell-msft I believe the repro would be a CMakeLists.txt with include_directories(SYSTEM path). I'm busy but @Colengms may be able to help get a complete repro if the user doesn't provide one.

sean-mcmanus avatar Jul 21 '23 20:07 sean-mcmanus

@arghness Could you please provide a repro sample that we could use to investigate this issue?

Additionally, where are the system header paths that are appearing in compile_commands and showing errors coming from? It'd be good to know where they are coming from, as it may be possible that CMake isn't aware of them because they are coming from a downstream tool.

Thanks!

My use-case is with Boost ( https://www.boost.org/ ) . In my case, I'm using a standard Boost install, but in a custom directory with a custom library suffix to avoid conflicts with the system version.

cmake_minimum_required(VERSION 3.0.0)
project(Test VERSION 0.1.0 LANGUAGES CXX)

list(APPEND CMAKE_PREFIX_PATH "/opt/foo/bar-boost/lib64/cmake")

find_package(Boost REQUIRED)
add_executable(Test main.cpp)

target_link_libraries(
  Test
  PRIVATE
    Boost::headers
)

This generates a compile_commands.json:

[
{
  "directory": "/foo/test_isystem/build",
  "command": "/usr/bin/g++  -isystem /opt/foo/bar-boost/include -g -o CMakeFiles/Test.dir/main.cpp.o -c /foo/test_isystem/main.cpp",
  "file": "/foo/test_isystem/main.cpp"
}
]

I also see in build/.cmake/api/v1/reply/target-Test-Debug-6d551a3dabb8405644ac.json :

	"compileGroups" : 
	[
		{
			"compileCommandFragments" : 
			[
				{
					"fragment" : "-g"
				}
			],
			"includes" : 
			[
				{
					"backtrace" : 2,
					"isSystem" : true,
					"path" : "/opt/foo/bar-boost/include"
				}
			],
			"language" : "CXX",
			"sourceIndexes" : 
			[
				0
			]
		}
	],

In the C++ extension, when I run "Run code analysis on active file", with "C_Cpp.loggingLevel": "Information", it ends with:

-I/opt/foo/bar-boost/include
-isystem/usr/include/c++/8
-isystem/usr/include/c++/8/x86_64-redhat-linux
-isystem/usr/include/c++/8/backward
-isystem/usr/lib/gcc/x86_64-redhat-linux/8/include
-isystem/usr/local/include
-isystem/usr/include

arghness avatar Jul 24 '23 17:07 arghness

@gcampbell-msft There are other cases, such as -iquote, -idirafter and -I-. One approach might be to never pass these in the includePath field and instead pass them in as arguments, making it the responsibility of the C/C++ Extension to handle the nuances.

Colengms avatar Jul 25 '23 20:07 Colengms

Here's an example where cmake tools isn't respecting SYSTEM PUBLIC.

Environment

  • OS and Version: Windows 11 23H2
  • VS Code Version: 1.90.2 (5437499feb04f7a586f677b155b039bc2b3669eb)
  • C/C++ Extension Version: v1.21.0 (pre-release)

Bug Summary and Steps to Reproduce

Bug Summary:

When using a cmake configuration that has a SYSTEM that inclusion isn't passed as system. This causes clang-tidy to analyze all external repositories.

Steps to reproduce:

  1. Create a cmake configuration that uses:
target_include_directories(${TARGET} SYSTEM PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/external/${REPO})
  1. Trace configuration passed

  2. See as clang/clang-tidy passes the include as not system, on build or tidy

Expected behavior:

System includes in cmake should be passed as system includes to clang/clang-tidy

Configuration and Logs

Custom configurations received:

{
  uri: {WORKSPACE_FILE}
  config: {
  "includePath": [
    "{WORKSPACE}/external/{REPO}"
  ],
  "compilerPath": "c:/program files/microsoft visual studio/2022/community/vc/tools/msvc/14.40.33807/bin/hostx64/x64/cl.exe",
  "compilerArgs": [],
  "compilerFragments": [
    "/DWIN32 /D_WINDOWS /GR /EHsc /O2 /Ob2 /DNDEBUG -std:c++20 -MD"
  ]
}
C:\Program Files\LLVM\bin\clang-tidy.exe
--export-fixes={LOCALAPPDATA}\Temp\{11890908921244206252}\fixes31200.yaml
--quiet
--use-color=false
{WORKSPACE}/src/file.cpp
--
-std=c++20
-xc++
-Wno-pragma-pack
-Wno-pragma-once-outside-header
-Ddevkit_EXPORTS
-DWIN32
-D_WINDOWS
-DNDEBUG
-D_MT
-D_DLL
-I{WORKSPACE}/external/{REPO}
-isystemC:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.40.33807/include
-isystemC:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.40.33807/atlmfc/include
-isystemC:/Program Files (x86)/Windows Kits/10/Include/10.0.26100.0/um
-isystemC:/Program Files (x86)/Windows Kits/10/Include/10.0.26100.0/ucrt
-isystemC:/Program Files (x86)/Windows Kits/10/Include/10.0.26100.0/shared
-isystemC:/Program Files (x86)/Windows Kits/10/Include/10.0.26100.0/winrt
-isystemC:/Program Files (x86)/Windows Kits/10/Include/10.0.26100.0/cppwinrt

Other Extensions

  • ms-vscode.cmake-tools

Additional context

  • https://clang.llvm.org/docs/UsersManual.html#controlling-diagnostics-in-system-headers
  • https://cmake.org/cmake/help/latest/command/target_include_directories.html

clshortfuse avatar Jul 05 '24 12:07 clshortfuse

@clshortfuse, Thanks for providing the above information. According to the information, we try to repro it using a simple demo project, we find that it included "'stddef.h' file. Please see the attached video. For further investigation, could you please share us a completed demo project with clear repro steps and then attach to this ticket? We are looking forward to hearing from you. Thanks.

ENV: Visual Studio Code version: 1.91.0 CMake Tools Extension version: v1.18.42 C/C++ Extension Version: v1.21.0 (pre-release) 071001

Evelyn-001 avatar Jul 10 '24 07:07 Evelyn-001

@Evelyn-001 Hi! Thanks for taking a look.

The issue is not that application does not build. The issue is that the "SYSTEM PUBLIC" items in the cmakelist.txt are passed as -I not -ISYSTEM.

This causes clang-tidy to perform its analysis on that file/folder. And for large repositories, it could mean the difference between 7 seconds and 7 minutes per build.

The project where I noticed the issue is this one:

https://github.com/clshortfuse/renodx/blob/9b230f9d9718279a189ca29355907aefc3d2b6d7/CMakeLists.txt#L102

We ended up having to remove clang-tidy entirely from our config file.

https://github.com/clshortfuse/renodx/commit/fad052116d6143c7aa72514d2b5a7f659a5e0a3b

For clarity, running cmake from the command line will not have this issue. It's only from the addon.

clshortfuse avatar Jul 10 '24 10:07 clshortfuse

We can repro this issue with below: ENV: Extension version: 1.19.19 VS Code version: Code 1.90.1 Repro steps:

  1. Clone the project and open it in VS code.
  2. Configure the project.
  3. Run C/C++: Run Code Analysis on All Files
  4. Observe the Debug logging information

Expected: Header path passed as -I.

Actual: Header path passed as -ISYSTEM. image

Evelyn-001 avatar Jul 12 '24 09:07 Evelyn-001

Hi @Evelyn-001 . To clarify, this issue is related to the "custom configuration" information provided by the CMake Tools extension, to the C/C++ Extension (which executes clang-tidy for code analysis). This API currently, unfortunately, has only a single entry for includePath, which does not provide a way to clearly distinguish which of them are user includes and which are system includes, and some features require this distinction. Instead, to work around that limitation, it would be necessary for CMake Tools to provide system includes via the compileArgs or compilerFragments fields (using the same arguments that would be passed to the compiler when building).

In fact, it would be best to always pass all include paths using their original arguments (to also properly handle -iquote, -idirafter, -I-, etc.) via the appropriate compiler arguments field of the custom configuration API, to preserve their original order and meaning, and not pass any of them through the includePaths field.

Colengms avatar Jul 15 '24 23:07 Colengms

Thanks for your reply. We will investigate this issue later and get back to you with any updates. Thank you for your support.

Evelyn-001 avatar Jul 16 '24 01:07 Evelyn-001