Skip to content

Atomic lint cpp tutorial

hgratten requested to merge atomic_lint_cpp_tutorial into master

this merge request introduces the clang-tidied c++ tutorial :

CppTutorial/*:

  • modernized headers (c to cpp)
  • reordered includes as good practice (user header first, then system headers, ordered alphabetically
  • wrapped body statement of if-else, for, do while, and while in braces. increases readability and forgetting braces is a common source of bugs for students
  • made class members private, this avoids public mutable members, which are a common source of bugs
  • removed "using namespace", since it clutters the translation unit, instead explicitly introduced individual symbols via using
  • marked functions that return something important with [[nodiscard]] attribute to throw a warning if result is not used
  • avoided uninitialized variables, instead used NAN to initialize doubles or -1 to initialize indices or nullptr for pointers. this is more likely to show up as an error than accidentally using unitialized memory
  • used emplace_back (e.g. for initializing sparse triplets) instead of push_back since it may save a copy
  • introduced const wherever possible
  • replaced long int by int_64t since long int has an implementation-defined size, which is inacceptable if guarantees are required
  • modernized function declaration
  • wrapped body statement of for, do while, and while in braces. increases readability and forgetting braces is a common source of bugs for students.
  • replaced default values in initialization list by default member initialization. this is safer and less repepetive when using multiple constructors
  • marked single-argument constructors explicit, to avoid accidental implicit conversion of an argument into class object
  • marked exception-free move constructor/assignment noexcept. as a good practice, this allows class users to recover safely (e.g. std::resize), otherwise it must copy
  • removed explicit return of std::move to allow compiler optimization to allow copy elision/return value optimization
  • modernized return statements of e.g. std::tuple with brace initialization instead of re-stating the return type to access its constructor
  • avoided pointer arithmetic, instead used span, which is safe to index
  • avoided else after return since it reduces readability of code flow and quickly causes bugs, instead used return codes or result variables and explicit break (single exit point)
  • replaced atoi with strtol since, atoi does not check the validity of the conversion and does not detect errors
  • avoided passing by value when passing by const reference is possible
  • explicitly casted between integer types (narrowing, different signedness)

CppTutorial/CMakeLists.txt:

  • removed cmake_minimum_required(VERSION 2.6.2), since the cmake version is better handled by the global CMakeLists.txt file
  • turned off clang-tidy and warnings for the cpptutorial code, since it also demonstrates what NOT to do (which obviously emits warnings)
  • changed to add_executable_numcse for consistency of targets and output paths

CppTutorial/cppdemo.cpp:

  • fixed concattest and ConcatIterator implementation
    • added special functions (destructor, move assignment, move constructor)
    • changed s1,s2 to pointers since std::max_element requires assigning to an iterator, which cannot be implemented with references
    • fixed post-increment operator, which was effectively a pre-increment operator
  • ignored locally modernize-loop-convert: here it is the idea to demonstrate the iterator use explicitly instead of using a range-based loop
  • replaced if-return-else-return by ternary operator
  • fixed X class implementation
    • used default constructor instead of writing it out
    • used gsl::owner to declare the X class an owner of the memory behind the data pointer. this is to avoid ownership warnings when allocating heap memory into data. gsl::owner is a purely syntactical type to communicate with static checkers and it is valid to implement it where needed instead of actually including the guideline support library
    • removed redundant nullptr check before delete
  • reordered print statement upwards in case 6, since we should not access x after a move

CppTutorial/cpptutorial.cpp:

  • removed using namespace, but left it as a comment with the existing explanation that it is bad style
  • fixed function return type
  • fixed typos
  • added comments
  • ignored locally misc-no-recursion, since we want to demonstrate how one function may call the other and vice-versa and how a forward declaration can solve this

CppTutorial/fnrefinclass.cpp:

  • ignored locally clang-analyzer-core.StackAddressEscape, since demonstrating this issue is the point of the code

CppTutorial/myvector.cpp:

  • reordered print statement upwards in case 6, since we should not access v1 after a move
  • used gsl::owner to declare the X class an owner of the memory behind the data pointer. this is to avoid ownership warnings when allocating heap memory into data. gsl::owner is a purely syntactical type to communicate with static checkers and it is valid to implement it where needed instead of actually including the guideline support library
  • ignored locally cppcoreguidelines-avoid-non-const-global-variables, since this is totally fine for turning on/off a debug flag
  • made sure every formal paremeter is named
  • avoided pointer arithmetic, instead used array indexing directly
  • removed redundant nullptr check before delete
  • aligned struct SimpleFunction which is inefficient to access if not properly aligned (as good practice). the alignment is determined by the static checker
  • replaced public member structsby private members and made structs publicly visible for users. instead used getter variables. this avoids public mutable members, which are a common source of bugs

CppTutorial/recorderdemo.cpp:

Merge request reports