-
Notifications
You must be signed in to change notification settings - Fork 0
17 Cpp Core Guidelines
Details of where, how C++ Core Guidlines checks are applied, and points of special interest are supplied on this page.
I recommend that you refer regularly to CppCoreGuidelines and
- find one you understand and agree with, then apply changes to your code to help you establish a new habit.
- find one you don't agree with, and discuss it with other engineers.
- find one you don't understand and research it until you do.
You can drive this process by looking at what Code Analysis identifies as dubious in your code base, but you can also read the guidelines, as these explain not only what to do, but also why it is important. Repeat this exercise regularly to improve your coding practice.
Regarding how App3Dev applies GSL, first note that the Nuget Microsoft GSL package (used in early versions of App3Dev) is deprecated, and contains an obsolete gsl. The current Microsoft GSL (version 4.0.0 at time of writing) is available on github as a vcpkg managed package.
However vcpkg is not installed with Visual Studio 2022 Community Edition (while Nuget is)... enough said.
App3Dev audience includes beginners who might struggle with setting vcpkg up, and certainly don't want to be forced into taking unnecessary extra steps before they can even assess their level of interest in the project material.
So to streamline the "getting started" procedure for App3Dev, the required gsl headers (current as of 28-12-2022) have simply been copied over into the App3Dev solution space and the (gsl) sub-directory has been added to the project include paths (See Properties -> C/C++ -> General -> Additional Include Directories). There are no libraries or linking steps required.
If you use App3Dev as a template for your own work, I recommend reversing these steps out (delete the include/gsl directory tree, and use vcpkg). Choose a more opportune moment in time to do this! Check microsoft/vcpkg and microsoft/GSL for instructions.
Enabling code analysis at build and choosing the ruleset "Microsoft All Rules" has been applied to all projects and all configurations in this solution. This applies all of the C++ Core Guidelines checks for all code with every build.
As a result of the steps above, the code has been reworked to apply the guidance wherever that is possible and appropriate.
What remains in the form of #pragma warning( disable : xxxxx ) can be searched for and will identify a few challenging issues that have been assessed (by me) as being acceptable "as-is". Without the benefit of peer review these remain a risk and an area for future attention.
Refer to stdafx.h in deliverable or test projects. Here it can be seen that _gsl.hpp is included. gsl.hpp is a wrapper that preserves the integrity of the supplied header file whilst disabling any warnings that the header itself raises. #include <gsl/gsl> implements safe features using language features that we are warned against using ourselves. Fair enough. I have used the wrapper file to disable the warnings that the gsl header would otherwise raise.
Refer to stdafx.h in test projects. A similar issue applies to CppUnitTest.h and spimpl.h and the response is the same (see CppUnitTest.hpp).
All other disabling of warnings are special cases where without the benefit of peer review I have assessed and made a judgment call that the warning is unwanted noise.
Visual Studio versions are continually improving with respect to false positives, and the App3Dev warning disabling is currently oriented to specific compiler version 17.0.5. If you are using a different version, the clean build experience might be impaired.
I first used static analysis (lint in 1985) when porting an embedded operating system written in 'C' to build with a different compiler. It saved me weeks. You may want to enable lint's successor Clang-Tidy. This tool is available in Visual Studio (but requires compiler versions 16.9.311 or later to work properly). I have enabled it for the x64 release build, and added a compile time warning message in case you are running an "older" compiler. This message explains that you can disable Clang-Tidy in the Code Analysis project settings, if you are not ready to upgrade your tools.
At time of writing 17.6.4 is available and I have used it to ensure that the project code compiles clean (also with a few supressed warnings that have been assessed as acceptable. search for // NOLINT comments ).
I know this advice will be set aside because of the impact on compile time. If you exercise discretion in this way, consider requiring clean build output with full static analysis on all builds and all platforms, and full unit test pass result with a pre-determined % test coverage as mandatory pre-conditions for checking-in new code.