Some C++ footgun avoidance.

Here is a list of footguns taken from my notes I found worthwhile to write up to remind myself not to do the same mistake again or how to workaround various compiler problems. It consists of type problems, easy to make memory issues mostly around uninitialized memory and stack-local values, low quality library code, problems with virtual classes, template code problems and some compiler specific problems. For a more complete view on the edge cases of the language, consider taking a look at the C++ iceberg and preprocessor iceberg.

Update 2024-12-17: Since the writing of this article (2024-04-15), I did significantly extend my CI-tested sample code on C++14, C++17, C++20 and C++23 and do not plan to continue to list footguns. My main complains about C++ are

  1. Concepts are not testable and there are no understandable backtraces for why concepts are not satisfied, which often makes them worse than templates.
  2. Simple things like renaming std::vector<bool> are not getting fixed.
  3. Many language features are unnecessary complex (function overloading, multiple constructors) instead of being usable via tagged union (std::variant).
  4. Individual declarative/functional parts can not be used by kernels via build system to speed up compilation.
  5. Compile time function execution is less powerful, but still offers no logical completeness or correctness advantage.
  6. Object oriented programming and virtual tables are not optional. There is no reason why string functions would require std::string or std::string_view instead of working on user-provided information, for example provided via struct with a fat pointer (pointer and length).

The current trajectory of C++ indicates to me that none of this will be fixed, even though modules would soon allow C++ version 2. Personally I would prefer semantics more comparable to Zig, but so far Zig has no static analysis to reduce transition efforts from C++.

  • Do not use hashmap[key], use auto search_hashmap = hashmap.find(); and write via iterator or use emplace, because there is no check for the elements existence or (typically raw C) values of members can remain undefined after object creation due to implicit default constructor.
    #include <map>
    #include <string>
    class T1 {
    public:
      T1(); // needed to allow convenient random access via [] operator
      T1(std::string const &t1)
          : mName(t1) {};
      std::string mName;
      std::string prop1;
    };
    class T2 {
    public:
      std::map<std::string, T1> mapex1;
      void AddT1(std::string const &t1str) {
        T1 t1obj(t1str);
        mapex1.emplace(t1str, t1obj);
        mapex1[t1str].prop1 = "blabla"; // potential footgun
      }
    };
    
    hashmap_simple_ub.cpp
  • Providing const char * to a function with reference will use the stack-local memory instead of using a copy. If further, c_str() is used to emplace into a std::map, this leads to UB due to usage of invalid memory once the stack local memory goes out of scope. In doubt, allocate a copy with std::string newstring = std::string(some_string) Especially in std::map or other owned containers. Only if there is an explicit comment on the storage including handling of move and copy constructor, use (const) char* as provided argument for (const) std::string &.
  • There is no type safety in reinterpret_cast usage, which is worse than memcpy. However, the prevention of another temporary for portable code may be worth it.
    #include <cstdint>
    int64_t usage_ptr(int64_t *val) { return *val; }
    int32_t reinterpret_cast_usage() {
      // clang-format: off
      uint8_t some_vals[9] = {0, 1, 0, 0, 0, 0, 0, 0, 0};
      // clang-format: on
      int64_t val_mcpy;
      // val_mcpy = memcpy(&val_mcpy, some_vals[1], sizeof(val_mcpy);  // refusal to compile
      val_mcpy = memcpy(&val_mcpy, &some_vals[1], sizeof(val_mcpy));    // refusal to compile
      int64_t val_wrong = *reinterpret_cast<int64_t *>(some_vals[1]);   // runtime error
      int64_t val_ok = *reinterpret_cast<int64_t *>(&some_vals[1]);     // correct usage
      int64_t *val_ok_ptr = reinterpret_cast<int64_t *>(&some_vals[1]); // correct usage
      usage_ptr(val_ok_ptr);
      usage_ptr(&val_mcpy); // necessary temporary copy hopefully elided
      if (val_ok != INT64_MIN)
        return 1;
      return 0;
    }
    
    reinterpret_cast_ts.cpp
  • C++ conversion string to int is worse without boost, so use C’s strtol.
  • Checking, if typename is a string is complex (even with C++17 extension).
    #include <string>
    #include <vector>
    template<typename STR> inline constexpr bool is_string_class_decayed = false;
    template<typename... STR> inline constexpr bool is_string_class_decayed<std::basic_string<STR...>> = true;
    // decay_t will remove const, & and volatile from the type
    template<typename STR>
    inline constexpr bool is_string_class =
        is_string_class_decayed<std::decay_t<STR>> template<typename TChar, typename TString>
        inline constexpr bool is_string = is_string_class<TString> && std::is_same_v<TChar, typename TString::value_type>;
    static_assert(is_string_class<std::string>);
    static_assert(is_string_class<std::wstring const &>); // that's why decay_t is needed
    static_assert(!is_string_class<int>);
    static_assert(!is_string_class<double const>);
    static_assert(!is_string_class<char const *>);
    static_assert(!is_string_class << std::vector < char >>);
    
    is_typname_string.hpp
  • auto does verbatim replacement of the return type, which can hide a stack-local copy or move. Only use auto for well-known iterators, status tuples, shared pointers etc, but never for objects and object references.
  • Interoperating type safe with c strings is cumbersome.
    void cstring_string_interop() {
      char const *cmd = "ls";
      char const *buffer[] = {"ls", "-l", nullptr};
      char *const *argv = const_cast<char *const *>(buffer);
      int execed = execve(cmd, argv, nullptr);
      (void)execed;
    }
    
    cstr_interop.cpp
  • MSVC has no reliable relative paths as macro yet (see experimental:deterministic mode). Workaround to get filename by Andry.
    #include <cstddef>
    #include <cstdint>
    #include <cstdio>
    template<typename T, size_t S> inline constexpr size_t fname_offs(T const (&str)[S], size_t i = S - 1) {
      return (str[i] == '/' || str[i] == '\\') ? i + 1 : (i > 0 ? fname_offs(str, i - 1) : 0);
    }
    template<typename T> inline constexpr size_t fname_offs(T (&str)[1]) { return 0; }
    namespace util_force_const_eval {
    template<typename T, T v> struct const_expr_value {
      static constexpr T const value = v;
    };
    } // namespace util_force_const_eval
    #define FORCE_CONST_EVAL(exp) ::util_force_const_eval::const_expr_value<decltype(exp), exp>::value
    #define LEAF(FN) (&FN[FORCE_CONST_EVAL(fname_offs(FN))])
    
    int testEq(int32_t a, int32_t b) {
      if (a != b) {
        // Prefer __FILE_NAME__, which also works in C. Ideally, the compiler
        // can be told to provide relative file paths.
        fprintf(stderr, "%s:%d got '%d' expected '%d'\n", LEAF(__FILE__), __LINE__, a, b);
        return 1;
      }
      return 0;
    }
    
    msvc_rel_path.cpp
  • Destructors of virtual classes should have lifetime annotation, unless final class. Otherwise, debugging problems will be painful, because the intended object lifetimes become ambiguous on reading the code. Consider using clang -Wnon-virtual-dtor or recent msvc (ca. year 2020).
  • Delete copy + move constructors for non-final classes with no pure-virtual methods.
  • Consider marking copy constructor as explicit and deleting copy assignment, if copying class is expensive. Consider providing a “clone fn” as syntactic sugar.
  • Do not use std::vector<bool>, because it is a dynamic bitset and has horrible naming. Accidental byte operations on it like memcpy will be UB.
  • Streams do not enforce C abi and are overly complex for printing memory. Further (no example here), they need to extensions to offer non-blocking reads, writes and introspection.
    #include <cstdint>
    #include <fstream>
    #include <iostream>
    #include <vector>
    void stream_cpp() {
      std::vector<uint8_t> array{1, 0, 0, 0};
      std::fstream fstream1{stdout, std::fstream::app | std::fstream::out};
      auto flags = fstream1.flags();
      fstream1 << std::hex;
      for (uint32_t i = 0; i < array.size(); i += 1)
        fstream1 << std::hex << array[i];
      fstream1.flags(flags);
      fstream1 << "\n";
      fstream1.close();
    }
    void fprintf_c() {
      std::vector<uint8_t> array{1, 0, 0, 0};
      for (uint32_t i = 0; i < array.size(); i += 1)
        fprintf(stdout, "%x, ", array[i]);
      fprintf(stdout, "\n");
    }
    
    streams_no_cabi.cpp
  • Incomplete type can not be instantiated in template:
    1. pull in all headers deps of headers
    2. class forward declares may create circular dependencies
    3. build each file individually
    4. might be a circular dependency during template usage
  • Missing virtual destructor for non-final methods in classes is technically UB.
    class ISomeInterface {
    public:
      virtual int SomeMethod() = 0;
    };
    class CSomeClass : ISomeInterface {
      CSomeClass();
    
    public:
      int SomeMethod() override { return 1; }
      virtual ~CSomeClass(); // missing virtual technical UB
    };
    class CSomeDerivedClass : CSomeClass {
      CSomeDerivedClass();
    
    public:
      int SomeMethod() override final { return 2; }
      virtual ~CSomeDerivedClass(); // optional virtual
    };
    
    missing_virt_destructor.cpp
  • Why exceptions do not scale.
    #include <cstdint>
    #include <stdexcept>
    int why_exceptions_dont_scale(char *errmsg_ptr, uint32_t *errmsg_len) {
      constexpr char const_drivermsg[] = "DriverError: ";
      constexpr char const_initmsg[] = "InitError: ";
      constexpr char const_nocamfoundmsg[] = "NoCameraFound: ";
      // Underlying idea: prefix exception strings with text. Below case handling
      // shows how error prone this is to make runtime decisions with C abi
      // compatibility across dll. And this does not cover compiler mangling and
      // dependency on runtime etc.
      // Further more, tooling like clangd is unable to infer all possible strings
      // to enforce correct error handling of the bubbled up exceptions and not even
      // all possible derived exception types.
      struct SomeDll some_dll;
      try {
        some_dll.SetupDll("someconfig_file");
      } catch (std::runtime_error &rt_err) {
        std::string err = rt_err.what();
        // std::string::StartsWith : err.rfind("DriverError:", 0) == 0)
        if (err.rfind(const_drivermsg, 0) == 0) {
          int st = snprintf(&errmsg_ptr[0], *errmsg_len, "%s", &err.c_str()[sizeof(const_drivermsg)]);
          if (st <= 0)
            return 1; // C89 allows less than 1, C99 NULL
          return 2;
        } else if (err.rfind(const_initmsg, 0) == 0) {
          int st = snprintf(&errmsg_ptr[0], *errmsg_len, "%s", &err.c_str()[sizeof(const_initmsg)]);
          if (st <= 0)
            return 1;
          return 2;
        } else if (err.rfind(const_nocamfoundmsg, 0) == 0) {
          int st = snprintf(&errmsg_ptr[0], *errmsg_len, "%s", &err.c_str()[sizeof(const_nocamfoundmsg)]);
          if (st <= 0)
            return 1;
          return 3;
        }
      } catch (std::exception &exc) {
        std::string err = exc.what();
        int st = snprintf(&errmsg_ptr[0], *errmsg_len, "%s", &err.c_str()[sizeof(const_nocamfoundmsg)]);
        if (st <= 0)
          return 1;
        return 100;
      }
      return 0;
    }
    
    exceptions_dont_scale.cpp
  • __STDC_VERSION__ is not necessary defined by C++ compilers.