Clang-tidy, VS Code, The Machinery... CMake?
2020-07-26Update (2020-07-29): The entire section on problems with build output can be solved by not using a workspace (*.code-workspace). Instead just use a typical settings.json in your .vscode folder. 🤘
For some developers the topic of static analysis is contentious. But I think it's an important tool to have in your toolbox especially when writing C or C++ code. Unfortunately the tools are either expensive or involve a lot of tip-toeing to avoid the false positives that tend to make developers toss their hands up and just live dangerously.
But for me I still think it's worth it. I'd rather get clang-tidy running decently enough and periodically adjust what checks are run, or to use a more extensive set of warnings than the typical default for a C++ compiler.
Rust put a lot of emphasis on warnings for things that wouldn't exactly make code unsound, but definitely helps you write code that is easier to change later. I think clang-tidy and -Wall -Wextra can help when writing C or C++ in the same way.
More involved static analysis tools are much better suited when you're writing safety-critical code that must conform to standards such as MISRA or AUTOSAR. And in those domains it appears to be relatively common to just use simulink and have it generate your C code and apply futher analysis and proofs after the fact. Or there is Ada/Spark that can be formally verified, then have C code generated from it.
But game development, tool development, is a creative endeavor and you can't formally prove that an artist or designer is empowered or inspired by your work.
And yet, I still feel like some basic tools can help keep me on a path to make my creative efforts more useful over a longer period of time. Our Machinery also belives that in some sense given that the use of clang-format is part of their design guidelines for code.
I asked about clang-tidy on the Our Machinery forums though and in general, the team aren't users or fans. Too many experiences with false positives and the like. Well, that won't stop me from trying a couple of things for my sake. :D
It's dangerous to go alone.. take this!
The easiest (and free) couple of tools would be cppcheck and clang-tidy.
Visual Studio can show you diagnostic output from the compiler, as well as from clang-tidy. Super great. VS Code has a couple of extensions for it's lint system that can display output from a wide variety of tools but unfortunately it doesn't appear that you can easily "stack" linters and have the output from multiple sources without building custom extensions.
Visual Studio 2019
I think that most users of Visual Studio are probably already aware of how the built-in linters work. And what's more, is that they're probably using Resharper or VAX. There is also an interesting looking Clang Power Tools extension but I haven't tried it yet.
Whether you're using CMake and "Open Folder" workflow, or using an msbuild project (as we end up doing when building The Machinery plugins since tmbuild uses premake to generate msbuild projects), the only thing you have to do to start seeing clang-tidy output is to set a project property for clang-tidy checks.
Documentation for setting it up is here.
I am curious though, since our projects are being generated via premake, after I set my lints up, they wouldn't survive a project refresh. Can premake support adding this property to a project?
There doesn't seem to be an official or otherwise widely used cppcheck extension for Visual Studio.
Output from builds, such as errors and warnings are so far all correctly parsed and shown in the "Error List" tool window and correctly navigate to the file and line which you can see here:
My simple test of these is to simply comment out the TM_SHUTDOWN_TEMP_ALLOCATOR(ta); at the end of the engine update function and verify that I get the intended warning that we're missing it.
Visual Studio Code
It probably comes as no surprise that there is a buffet of extensions for using clang tools in VS Code. I chose to go with the clang-tidy extension. And I was not disappointed after getting over some configuration hurdles.
A second extension that I tried out and was fairly pleased with was the C/C++ Advanced Lint extension. Of particular interest is that it supports cppcheck. It also support "clang" but not clang-tidy which is a choice.
I'll avoid a rather long and windy exposition about how I arrived at the configuration I'm using and just place it here so you can refer to at your leisure:
{
"clang-tidy.compilerArgs": [
"-DTM_OS_WINDOWS",
"-IOurMachinery/headers"
],
"clang-tidy.compilerArgsBefore": [
"-W-std=c11",
"-Wall",
"-Wextra",
"-Weverything",
"-Wno-unused-local-typedef",
"-Wno-missing-braces",
"-Wno-microsoft-anon-tag",
"-Wno-unused-command-line-argument"
],
"clang-tidy.blacklist": [
"foundation/.*" // Really don't want to be seeing output from The Machinery headers we can't control.
],
"clang-tidy.checks": [
"bugprone-*",
"-bugprone-branch-clone", // TM_STATIC_HASH
"clang-analyzer-*",
"cert-*",
"cppcoreguidelines-init-variables",
"hicpp-signed-bitwise",
"misc-*",
"performance-*",
"portability-*",
"readability-*",
"-readability-uppercase-literal-suffix"
]
}
This config provides decent feedback when coding. Clang-tidy isn't the fastest in the world at running checks (well, it's running a lot of checks). By including the warnings that I'd otherwise be using during a build I'm getting feedback early as well. I like to work with -Werror when possible so having some feedback early on helps. You can see an example of the results here:
Pressing F8 will jump through the problem list and visit the files affected. Very nice!
What about cppcheck?
cppcheck seems to have a lot going for it and I'll probably spend some more time with it. Once example where it digs a tad deeper than clang-tidy is that in the TM_INIT_TEMP_ALLOCATOR(ta); macro it notices and reports that the helpfully named variable used to give us a warning regarding the missing shutdown call, is actually not being initialized.
The problem for me comes with the carray utils that are a part of The Machinery. These trigger a lot of errors around null pointers and the like. They are very likely false positives since the code demonstrably works. But you can't fault cppcheck for being upset that you basically take a nullpointer.. then push a value on to it via a very nested set of macros.
I assume there is a test suite being run before releases and so on that would catch any regressions and the code probably isn't being touched much... but it's a problem for me that we get FP on it since the "lib" is essentially a collection of macros that you inline to your compilation unit to use. Which means I'd be getting these errors quite a bit. And the checks themselves I wouldn't want to disable.
Unfortunately I can't submit this code to the cppcheck project as a false positive example. I might try and replicate it with a set of example macros... but at some point all you're doing is giving in to the itch and not learning or doing the things I wish I would be.
Sticking with clang-tidy for now.
For the moment I'm happy with what I'm getting from clang-tidy and VS Code. I should say that, cppcheck works pretty great and quick. The reason I didn't want to use it was because of it's results from looking into code that's essentially out of my control.
Unused parameters are things I like to have warnings for. Also, unusued field initializers. The Machinery template premake script is disabling these, which I'll leave as-is, but at least with clang-tidy I'll get my precious squiggles. To deal with missing field initializers one option is to use the nice C syntax for named field initialization. For unused parameters, either my interface is poorly specified, or I'm implementing an interface for The Machinery and I literally don't plan or need to use some of the parameters. For that case I have the following solution:
#define UNUSED(x) (void)(x)
Some folks might be annoyed with this. Fine. :D But to me this is a very direct way to letting future readers of the code that, yes, this parameter is indeed intentionally unused.
But what about build output?
With VS2019 we're getting correctly parsed output from msbuild so that you see both the linter warning and the build error (since I'm using -Werror). But in VS Code this hasn't worked for me yet. Something is not quite right with the output from the build and the result is that errors, or warnings, are not associated with any workspace files. Nor can I ctrl+click the paths:
A very clear issue can be seen when I do try to ctrl+click the path in the output.
My guess is that, this is a problem with project structure. I have the premake script in a subfolder of the workspace, and I must run the build using the following task definition:
{
"version": "2.0.0",
"type": "shell",
"tasks": [
{
"label": "Build CosmicTrash",
"group": {
"kind": "build",
"isDefault": true
},
"windows": {
"command": "tmbuild.exe -c Release --clang"
},
"args": [],
"problemMatcher": [
"$msCompile"
],
"options": {
"cwd": "${workspaceFolder}/CosmicTrash"
}
}
],
}
I feel like, if the build output would use absolute paths then we wouldn't have an issue. But the premake generated project files are using relative paths from the build directory they create.
<ItemGroup>
<ClCompile Include="..\..\plugins\sprite_component\sprite_component.c" />
</ItemGroup>
I don't see much in the docs regarding what options I have for dealing with this. But after moving the project around and trying to build from another location I at least narrowed one thing down. There is a location key in the project definition in our premake script. It's value was build/sprite_component and this now becomes clear how we could try and move forward.
If I set the location of the project to "." then I can ctrl+click on errors or warnings. Not because I fixed the issue though. It's just a coincidence of project location. Because instead of ../.. we now have .. and since we're running from a subdirectory it ends up correctly finding the file... but only when using ctrl+click. The error matcher still can't figure out where this file is.
hrm... So I guess... maybe it works from the workspace root? NOPE.
Does the error matcher work at all? Is it ever supposed to be output to the Problems tool window?
At this point I'm getting fed up. I'm intensely unsatisfied that I can't reach Emacs levels of compilation output functionality (using a shortcut to cycle through compilation warnings and errors).
Could this be, because we're using clang-cl? Does msvc output paths differently?
First I'll move things back into a subdirectory because random build files everywhere suuuuuuuucks.
My directory structure now looks like:
<workspace>/
|_ CosmicTrash/
|_|_ premake5.lua
Working directory for the build is CosmicTrash, project location in the premake script is ".".
Output from clang-cl: plugins\sprite_component\sprite_component.c(85,5).
Output from msvc: C:\projects\slowgames\CosmicTrash\plugins\sprite_component\sprite_component.c(85,5)
WHOA WHOA WHOA wait a second... has this all been a misdirection because I'm using clang? And I still dont get compiler matches... so does compiler output ever get sent to the problems window?
Let's back up here. Let's roll this all the way back to the beginning. Did msvc always give absolute paths?
C:\projects\slowgames\CosmicTrash>tmbuild -c Release --clang
Building configurations...
Running action 'vs2019'...
Generated CosmicTrash.sln...
Generated build/sprite_component/sprite_component.vcxproj...
Done (74ms).
Microsoft (R) Build Engine version 16.6.0+5ff7b0c9e for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.
..\..\plugins\sprite_component\sprite_component.c(85,5): error : unused variable 'ta_TM_SHUTDOWN_TEMP_ALLOCATOR_is_missing' [-Werror,-Wunused-variable] [C:\projects\slowgames\CosmicTrash\build
\sprite_component\sprite_component.vcxproj]
C:\projects\slowgames\OurMachinery\headers\foundation/temp_allocator.h(98,14): message : expanded from macro 'TM_INIT_TEMP_ALLOCATOR' [C:\projects\slowgames\CosmicTrash\build\sprite_component\
sprite_component.vcxproj]
<scratch space>(28,1): message : expanded from here [C:\projects\slowgames\CosmicTrash\build\sprite_component\sprite_component.vcxproj]
C:\projects\slowgames\CosmicTrash>rm -rf CosmicTrash.sln build
C:\projects\slowgames\CosmicTrash>tmbuild -c Release
Building configurations...
Running action 'vs2019'...
Generated CosmicTrash.sln...
Generated build/sprite_component/sprite_component.vcxproj...
Done (75ms).
Microsoft (R) Build Engine version 16.6.0+5ff7b0c9e for .NET Framework
Copyright (C) Microsoft Corporation. All rights reserved.
sprite_component.c
C:\projects\slowgames\CosmicTrash\plugins\sprite_component\sprite_component.c(85,5): error C2220: the following warning is treated as an error [C:\projects\slowgames\CosmicTrash\build\sprite_c
omponent\sprite_component.vcxproj]
C:\projects\slowgames\CosmicTrash\plugins\sprite_component\sprite_component.c(85,5): warning C4101: 'ta_TM_SHUTDOWN_TEMP_ALLOCATOR_is_missing': unreferenced local variable [C:\projects\slowgam
es\CosmicTrash\build\sprite_component\sprite_component.vcxproj]
C:\projects\slowgames\CosmicTrash>
Bruh
Y'all just made me go all out
My quest to live the zen of tmbuild has faltered. Let's whip out a 10 line cmake script and see what we get when we do. Because I'm just convinced that it'll work where tmbuild/premake is failing me.
Lemme go on record and just wonder out loud why people are avoiding CMake in favor of less functional tools that just bake msbuild projects.... when even Microsoft is going hard on cross-platform builds with CMake?
Do I love CMake? Nope. It's a dumpster fire that does waaaaay too much. It gets better with every release but there are volumes of old and bad, and I'm not sure how to take the new conventions and make them policy. So I 100% understand where people are coming from when they learn to avoid using it.
On the other hand... it's the closest thing we get to a standard build system for C and C++. It does the damned job and is supported as a native project model for basically every IDE or editor used by the community at large. It works everywhere (that I know of) and can be bootstrapped from tools pretty easily thanks to releases including "portable" options in self-contained archives.
The Machinery is written in C. The premake script is mostly just setting some common settings and ensuring the output of the build goes into the TM_SDK_DIR/bin/plugins. We can replicate this for great justice and I'm sure everybody reading is straight up:
Hold my beer
So I'm talking a lot of shit right now, but we can test really quickly whether there is even a chance this actually works. We don't even have to try and replicate the premake build because a single shared library target which can't find the SDK will give us error output and we'll see if it shows up.
First step, install the cmake tools extension.
Second step is to add a CMakeLists.txt to the workspace root:
cmake_minimum_required(VERSION 3.15)
project(Slowgames VERSION 0.1.0)
add_library(cosmic_trash SHARED CosmicTrash/plugins/sprite_component/sprite_component.c)
Third step, let the extension configure using the "Visual Studio Community 2019 - amd64" kit. Use the "Release" variant. All of which is just available from the toolbar: 
Finally we build, knowing full well we'll get an error... aaaaand.... YES:
The CMake tools for VS Code do not use the terminal and it bundles a suite of output parsers that apparently work.
However, if I configure the build with clang it can't parse the output...
So let's check the score:
| CMake Build | CMake - tasks.json | tmbuild | |
|---|---|---|---|
| MSVC | List ✔ Click ✖ | List ✖ Click ✔ | List ✖ Click ✔ |
| CLANG | List ✖ Click ✖ | List ✖ Click ✖ | List ✖ Click ✖ |
For a moment I wondered whether I'm just barking up the wrong tree and the problem matcher does actually work since the output is colored by warning and error severity. Removing the warning matcher from the task however didn't change the fact that output is colored.
I've changed the task type from "shell" to "process". I've run builds using cl.exe directly as well. No dice. Either the standard cpp tools in VS Code are currently broken and not working as documented. Or something beyond my capacity to deal with right now is the root of the problem.
Where I'm at, at the end of all this...
I'm wrong. tmbuild is all fine and dandy. It's the environment I'm working in that conspires against me. VS Code just isn't working. Not even their own documentation works correctly. Even if you have a simple task that runs cl.exe on a file containing an intentional error, the $msCompile problem matcher will not give you what you need. (circa 1.47.3)
Meanwhile, not being able to ctrl+click the path with clang, that's sorta fixed by adding -fdiagnostics-absolute-paths to the clang options in premake5.lua. 👍 I say sorta because it works sometimes but not always.
What I want most is to have the output from the build included in the Problems tool window. It appears that my only option would be to use CMake and MSVC until sometime in the future when any of this works again.
For those curious, here is the start of the_machinery.cmake:
if (NOT TM_SDK_DIR)
message(FATAL "TM_SDK_DIR is not set!")
endif()
set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>")
set(TM_SDK_HEADERS_DIR ${TM_SDK_DIR}/headers)
set(TM_SDK_PLUGINS_DIR ${TM_SDK_DIR}/bin/plugins)
macro(tm_plugin target_name)
add_library(${target_name} SHARED ${ARGN})
set_target_properties(${target_name} PROPERTIES
C_STANDARD 11
OUTPUT_NAME "tm_${target_name}"
RUNTIME_OUTPUT_DIRECTORY "${TM_SDK_PLUGINS_DIR}"
)
if (WIN32)
target_compile_definitions(${target_name} PRIVATE "TM_OS_WINDOWS" "_CRT_SECURE_NO_WARNINGS")
if (CMAKE_BUILD_TYPE MATCHES "Debug")
target_compile_definitions(${target_name} PRIVATE "TM_CONFIGURATION_DEBUG" "DEBUG")
else()
target_compile_definitions(${target_name} PRIVATE "TM_CONFIGURATION_RELEASE" "_DEBUG" "NDEBUG")
endif()
if (MSVC)
target_compile_options(${target_name} PRIVATE
/W4 /WX
/wd4057 # Slightly different base types. Converting from type with volatile to without.
/wd4100 # Unused formal parameter. I think unusued parameters are good for documentation.
/wd4152 # Conversion from function pointer to void *. Should be ok.
/wd4200 # Zero-sized array. Valid C99.
/wd4201 # Nameless struct/union. Valid C11.
/wd4204 # Non-constant aggregate initializer. Valid C99.
/wd4206 # Translation unit is empty. Might be #ifdefed out.
/wd4214 # Bool bit-fields. Valid C99.
/wd4221 # Pointers to locals in initializers. Valid C99.
/wd4702 # Unreachable code. We sometimes want return after exit() because otherwise we get an error about no return value.
)
target_link_options(${target_name} PRIVATE /ignore:4099)
endif()
endif()
if(${CMAKE_C_COMPILER_ID} MATCHES "Clang")
target_compile_options(${target_name} PRIVATE
-Wno-missing-field-initializers # = {0} is OK.
-Wno-unused-parameter # Useful for documentation purposes.
-Wno-unused-local-typedef # We don't always use all typedefs.
-Wno-missing-braces # = {0} is OK.
-Wno-microsoft-anon-tag # Allow anonymous structs.
-Wno-unused-command-line-argument # stop complainging about the /std:c++14 option
-Werror
-fdiagnostics-absolute-paths
-fms-extensions # Allow anonymous struct as C inheritance.
-mavx # AVX.
-mfma
)
endif()
target_include_directories(${target_name} PRIVATE "${TM_SDK_DIR}/headers")
endmacro(tm_plugin)
Then for your projects you're CMakeLists.txt might look something like this:
cmake_minimum_required(VERSION 3.15)
project(Slowgames VERSION 0.1.0)
set(TM_SDK_DIR ${PROJECT_SOURCE_DIR}/OurMachinery)
include(${TM_SDK_DIR}/the_machinery.cmake)
tm_plugin(cosmic_trash
cosmic_trash/src/foo.h
cosmic_trash/src/bar.inl
cosmic_trash/src/foo.c)
The function tm_plugin could easily include a post-build command to run docgen, and even a pre-build command to run hash.