Enable clang-tidy core guideline checks (#971)

This is another PR to enable a greater set of `clang-tidy` checks; as
usual it can be reviewed commit-by-commit to see the changes that fix a
particular error from the checker (with the exception of
525da5a,
which is an enabling refactoring that's big enough to warrant its own
separate commit).

.clang-tidy

@@ -1,6 +1,7 @@
 Checks:
   clang-analyzer-*
   -clang-analyzer-cplusplus.PlacementNew
+  -clang-analyzer-optin.cplusplus.UninitializedObject
   misc-*
   -misc-no-recursion
   -misc-unused-parameters
@@ -19,7 +20,17 @@ Checks:
   -bugprone-narrowing-conversions
   -bugprone-exception-escape
   -bugprone-implicit-widening-of-multiplication-result
-  -bugprone-unchecked-optional-access # false positives
+  -bugprone-unchecked-optional-access
+  cppcoreguidelines-*
+  -cppcoreguidelines-avoid-magic-numbers
+  -cppcoreguidelines-avoid-non-const-global-variables
+  -cppcoreguidelines-narrowing-conversions
+  -cppcoreguidelines-no-malloc
+  -cppcoreguidelines-owning-memory
+  -cppcoreguidelines-pro-bounds-array-to-pointer-decay
+  -cppcoreguidelines-pro-bounds-pointer-arithmetic
+  -cppcoreguidelines-pro-type-cstyle-cast
+  -cppcoreguidelines-pro-type-union-access
 
 WarningsAsErrors:
   '*'
@@ -29,3 +40,5 @@ CheckOptions:
     value: false
   - key: 'readability-function-cognitive-complexity.IgnoreMacros'
     value: true
+  - key: 'cppcoreguidelines-macro-usage.CheckCapsOnly'
+    value: true

bindings/python/runtime.cpp

@@ -81,8 +81,8 @@ void bind_runtime(py::module_ &m) {
       .def(
           "serialize",
           [](block *term, bool emit_size) {
-            char *data;
-            size_t size;
+            char *data = nullptr;
+            size_t size = 0;
             serializeConfiguration(term, nullptr, &data, &size, emit_size);
             return py::bytes(std::string(data, data + size));
           },

include/kllvm/codegen/CreateTerm.h

@@ -32,8 +32,8 @@ class CreateTerm {
       KORECompositePattern *constructor, llvm::Value *val,
       std::string locationStack = "0");
   bool populateStaticSet(KOREPattern *pattern);
-  std::pair<llvm::Value *, bool>
-  createAllocation(KOREPattern *pattern, std::string locationStack = "0");
+  std::pair<llvm::Value *, bool> createAllocation(
+      KOREPattern *pattern, std::string const &locationStack = "0");
 
 public:
   CreateTerm(

include/runtime/header.h

@@ -9,6 +9,8 @@
 #include <gmp.h>
 #include <mpfr.h>
 
+#include <fmt/printf.h>
+
 #include "config/macros.h"
 #include "runtime/alloc.h"
 #include "runtime/fmt_error_handling.h"
@@ -384,8 +386,6 @@ void printList(
     writer *, list *, char const *, char const *, char const *, void *);
 void visitChildren(block *subject, writer *file, visitor *printer, void *state);
 
-void sfprintf(writer *, char const *, ...);
-
 stringbuffer *hook_BUFFER_empty(void);
 stringbuffer *hook_BUFFER_concat(stringbuffer *buf, string *s);
 stringbuffer *
@@ -422,4 +422,15 @@ std::string intToString(mpz_t);
 void printValueOfType(
     std::ostream &os, std::string const &definitionPath, void *value,
     std::string const &type);
+
+template <typename... Args>
+void sfprintf(writer *file, char const *fmt, Args &&...args) {
+  if (file->file) {
+    fmt::fprintf(file->file, fmt, args...);
+  } else {
+    auto str = fmt::sprintf(fmt, args...);
+    hook_BUFFER_concat_raw(file->buffer, str.data(), str.size());
+  }
+}
+
 #endif // RUNTIME_HEADER_H

lib/ast/AST.cpp

@@ -191,7 +191,7 @@ std::string KORECompositeSort::getHook(KOREDefinition *definition) const {
 }
 
 ValueType KORECompositeSort::getCategory(std::string const &hookName) {
-  SortCategory category;
+  SortCategory category = SortCategory::Uncomputed;
   uint64_t bits = 0;
   if (hookName == "MAP.Map") {
     category = SortCategory::Map;

lib/ast/definition.cpp

@@ -12,7 +12,7 @@ std::unordered_map<Elem *, std::unordered_set<Elem *, Hash, Equal>, Hash, Equal>
 transitiveClosure(std::unordered_map<
                   Elem *, std::unordered_set<Elem *, Hash, Equal>, Hash, Equal>
                       relations) {
-  bool dirty;
+  bool dirty = false;
   do {
     dirty = false;
     for (auto &entry : relations) {

lib/binary/serializer.cpp

@@ -7,7 +7,7 @@ namespace detail {
 
 bool is_big_endian() {
   uint32_t i = 1;
-  auto *c = reinterpret_cast<uint8_t *>(&i);
+  auto *c = static_cast<uint8_t *>(static_cast<void *>(&i));
   return *c == 0x00;
 }
 
@@ -29,8 +29,14 @@ serializer::serializer(flags f)
 }
 
 std::string serializer::byte_string() const {
-  auto const *ptr = reinterpret_cast<unsigned char const *>(buffer_.data());
-  return {ptr, ptr + buffer_.size()};
+  auto ret = std::string{};
+  ret.reserve(buffer_.size());
+
+  for (auto byte : buffer_) {
+    ret.push_back(static_cast<char>(byte));
+  }
+
+  return ret;
 }
 
 void serializer::reset() {

lib/codegen/CreateStaticTerm.cpp

@@ -60,7 +60,7 @@ llvm::Constant *CreateStaticTerm::notInjectionCase(
 
     int idx = 2;
     for (auto const &child : constructor->getArguments()) {
-      llvm::Constant *ChildValue;
+      llvm::Constant *ChildValue = nullptr;
       if (idx++ == 2 && val != nullptr) {
         ChildValue = val;
       } else {
@@ -200,8 +200,8 @@ CreateStaticTerm::createToken(ValueType sort, std::string contents) {
                                  Module->getContext(), FLOAT_WRAPPER_STRUCT));
     auto *globalVar = llvm::dyn_cast<llvm::GlobalVariable>(global);
     if (!globalVar->hasInitializer()) {
-      size_t prec;
-      size_t exp;
+      size_t prec = 0;
+      size_t exp = 0;
       char const last = contents.back();
       if (last == 'f' || last == 'F') {
         prec = 24;
@@ -222,7 +222,7 @@ CreateStaticTerm::createToken(ValueType sort, std::string contents) {
       }
       mpfr_t value;
       mpfr_init2(value, prec);
-      int retValue;
+      int retValue = 0;
       if (contents == "+Infinity" || contents == "-Infinity"
           || contents == "Infinity") {
         retValue = mpfr_set_str(value, contents.c_str(), 10, MPFR_RNDN);

lib/codegen/CreateTerm.cpp

@@ -462,7 +462,7 @@ llvm::Value *CreateTerm::createHook(
         llvm::ConstantInt::get(llvm::Type::getInt64Ty(Ctx), nwords * 8),
         CurrentBlock, "koreAllocAlwaysGC");
     if (nwords == 1) {
-      llvm::Value *Word;
+      llvm::Value *Word = nullptr;
       if (cat.bits == 64) {
         Word = mint;
       } else {
@@ -514,7 +514,7 @@ llvm::Value *CreateTerm::createHook(
         llvm::ConstantInt::get(llvm::Type::getInt64Ty(Ctx), nwords * 8),
         CurrentBlock, "koreAllocAlwaysGC");
     if (nwords == 1) {
-      llvm::Value *Word;
+      llvm::Value *Word = nullptr;
       if (cat.bits == 64) {
         Word = mint;
       } else {
@@ -717,13 +717,13 @@ llvm::Value *CreateTerm::createFunctionCall(
   case SortCategory::Set: collection = true; break;
   default: sret = false; break;
   }
-  llvm::Value *AllocSret;
+  llvm::Value *AllocSret = nullptr;
   types.reserve(args.size());
   for (auto *arg : args) {
     types.push_back(arg->getType());
   }
   std::vector<llvm::Value *> realArgs = args;
-  llvm::Type *sretType;
+  llvm::Type *sretType = nullptr;
   if (sret) {
     // we don't use alloca here because the tail call optimization pass for llvm
     // doesn't handle correctly functions with alloca
@@ -769,7 +769,7 @@ llvm::Value *CreateTerm::notInjectionCase(
   int idx = 2;
   std::vector<llvm::Value *> children;
   for (auto const &child : constructor->getArguments()) {
-    llvm::Value *ChildValue;
+    llvm::Value *ChildValue = nullptr;
     if (idx == 2 && val != nullptr) {
       ChildValue = val;
     } else {
@@ -856,8 +856,8 @@ bool CreateTerm::populateStaticSet(KOREPattern *pattern) {
   return can_be_static;
 }
 
-std::pair<llvm::Value *, bool>
-CreateTerm::createAllocation(KOREPattern *pattern, std::string locationStack) {
+std::pair<llvm::Value *, bool> CreateTerm::createAllocation(
+    KOREPattern *pattern, std::string const &locationStack) {
   if (staticTerms.count(pattern)) {
     auto *staticTerm = new CreateStaticTerm(Definition, Module);
     return (*staticTerm)(pattern);
@@ -1257,10 +1257,10 @@ bool isCollectionSort(ValueType cat) {
   }
 }
 
-bool isInjectionSymbol(KOREPattern *p, KORESymbol *inj) {
+bool isInjectionSymbol(KOREPattern *p, KORESymbol *sym) {
   if (auto *constructor = dynamic_cast<KORECompositePattern *>(p)) {
     KORESymbol const *symbol = constructor->getConstructor();
-    if (symbol->getName() == inj->getName()) {
+    if (symbol->getName() == sym->getName()) {
       return true;
     }
   }

lib/codegen/Debug.cpp

@@ -165,16 +165,16 @@ llvm::DIType *getDebugType(ValueType type, std::string const &typeName) {
     return nullptr;
   }
   static std::map<std::string, llvm::DIType *> types;
-  llvm::DIType *map;
-  llvm::DIType *rangemap;
-  llvm::DIType *list;
-  llvm::DIType *set;
-  llvm::DIType *integer;
-  llvm::DIType *floating;
-  llvm::DIType *buffer;
-  llvm::DIType *boolean;
-  llvm::DIType *mint;
-  llvm::DIType *symbol;
+  llvm::DIType *map = nullptr;
+  llvm::DIType *rangemap = nullptr;
+  llvm::DIType *list = nullptr;
+  llvm::DIType *set = nullptr;
+  llvm::DIType *integer = nullptr;
+  llvm::DIType *floating = nullptr;
+  llvm::DIType *buffer = nullptr;
+  llvm::DIType *boolean = nullptr;
+  llvm::DIType *mint = nullptr;
+  llvm::DIType *symbol = nullptr;
   if (types[typeName]) {
     return types[typeName];
   }

lib/codegen/Decision.cpp

@@ -158,7 +158,7 @@ void SwitchNode::codegen(Decision *d) {
   bool isInt = false;
   for (auto &_case : cases) {
     auto *child = _case.getChild();
-    llvm::BasicBlock *CaseBlock;
+    llvm::BasicBlock *CaseBlock = nullptr;
     if (child == FailNode::get()) {
       CaseBlock = d->FailureBlock;
     } else {
@@ -187,8 +187,8 @@ void SwitchNode::codegen(Decision *d) {
     val = cmp;
     isInt = true;
   }
-  llvm::Value *failSort;
-  llvm::Value *failPattern;
+  llvm::Value *failSort = nullptr;
+  llvm::Value *failPattern = nullptr;
   if (d->FailPattern) {
     auto failReason = getFailPattern(caseData, isInt, d->FailureBlock);
     failSort = d->stringLiteral(failReason.first);
@@ -241,7 +241,7 @@ void SwitchNode::codegen(Decision *d) {
       KORESymbolDeclaration *symbolDecl
           = d->Definition->getSymbolDeclarations().at(
               _case.getConstructor()->getName());
-      llvm::Instruction *Renamed;
+      llvm::Instruction *Renamed = nullptr;
       for (auto const &binding : _case.getBindings()) {
         llvm::Value *ChildPtr = llvm::GetElementPtrInst::CreateInBounds(
             BlockType, Cast,
@@ -250,7 +250,7 @@ void SwitchNode::codegen(Decision *d) {
                  llvm::Type::getInt32Ty(d->Ctx), offset + 2)},
             "", d->CurrentBlock);
 
-        llvm::Value *Child;
+        llvm::Value *Child = nullptr;
         auto cat = dynamic_cast<KORECompositeSort *>(
                        _case.getConstructor()->getArguments()[offset].get())
                        ->getCategory(d->Definition);
@@ -413,7 +413,7 @@ void FunctionNode::codegen(Decision *d) {
   std::vector<llvm::Value *> args;
   llvm::StringMap<llvm::Value *> finalSubst;
   for (auto [arg, cat] : bindings) {
-    llvm::Value *val;
+    llvm::Value *val = nullptr;
     if (arg.first.find_first_not_of("-0123456789") == std::string::npos) {
       val = llvm::ConstantInt::get(
           llvm::Type::getInt64Ty(d->Ctx), std::stoi(arg.first));
@@ -771,9 +771,9 @@ void makeEvalOrAnywhereFunction(
   llvm::BasicBlock *fail
       = llvm::BasicBlock::Create(module->getContext(), "fail", matchFunc);
 
-  llvm::AllocaInst *choiceBuffer;
-  llvm::AllocaInst *choiceDepth;
-  llvm::IndirectBrInst *jump;
+  llvm::AllocaInst *choiceBuffer = nullptr;
+  llvm::AllocaInst *choiceDepth = nullptr;
+  llvm::IndirectBrInst *jump = nullptr;
   initChoiceBuffer(
       dt, module, block, stuck, fail, &choiceBuffer, &choiceDepth, &jump);
 
@@ -800,7 +800,7 @@ void abortWhenStuck(
   auto &Ctx = Module->getContext();
   symbol = d->getAllSymbols().at(ast_to_string(*symbol));
   auto *BlockType = getBlockType(Module, d, symbol);
-  llvm::Value *Ptr;
+  llvm::Value *Ptr = nullptr;
   auto *BlockPtr = llvm::PointerType::getUnqual(
       llvm::StructType::getTypeByName(Module->getContext(), BLOCK_STRUCT));
   if (symbol->getArguments().empty()) {
@@ -1056,7 +1056,7 @@ void makeStepFunction(
   auto *blockType = getValueType({SortCategory::Symbol, 0}, module);
   auto *debugType
       = getDebugType({SortCategory::Symbol, 0}, "SortGeneratedTopCell{}");
-  llvm::FunctionType *funcType;
+  llvm::FunctionType *funcType = nullptr;
   std::string name;
   if (search) {
     name = "stepAll";
@@ -1088,9 +1088,9 @@ void makeStepFunction(
   llvm::BasicBlock *fail
       = llvm::BasicBlock::Create(module->getContext(), "fail", matchFunc);
 
-  llvm::AllocaInst *choiceBuffer;
-  llvm::AllocaInst *choiceDepth;
-  llvm::IndirectBrInst *jump;
+  llvm::AllocaInst *choiceBuffer = nullptr;
+  llvm::AllocaInst *choiceDepth = nullptr;
+  llvm::IndirectBrInst *jump = nullptr;
 
   initChoiceBuffer(
       dt, module, block, pre_stuck, fail, &choiceBuffer, &choiceDepth, &jump);
@@ -1207,9 +1207,9 @@ void makeMatchReasonFunction(
       {FailSubject, FailPattern, FailSort}, "", fail);
   setDebugLoc(call);
 
-  llvm::AllocaInst *choiceBuffer;
-  llvm::AllocaInst *choiceDepth;
-  llvm::IndirectBrInst *jump;
+  llvm::AllocaInst *choiceBuffer = nullptr;
+  llvm::AllocaInst *choiceDepth = nullptr;
+  llvm::IndirectBrInst *jump = nullptr;
   initChoiceBuffer(
       dt, module, block, pre_stuck, fail, &choiceBuffer, &choiceDepth, &jump);
 
@@ -1303,9 +1303,9 @@ void makeStepFunction(
   llvm::BasicBlock *fail
       = llvm::BasicBlock::Create(module->getContext(), "fail", matchFunc);
 
-  llvm::AllocaInst *choiceBuffer;
-  llvm::AllocaInst *choiceDepth;
-  llvm::IndirectBrInst *jump;
+  llvm::AllocaInst *choiceBuffer = nullptr;
+  llvm::AllocaInst *choiceDepth = nullptr;
+  llvm::IndirectBrInst *jump = nullptr;
   initChoiceBuffer(
       res.dt, module, block, pre_stuck, fail, &choiceBuffer, &choiceDepth,
       &jump);

lib/codegen/DecisionParser.cpp

@@ -76,7 +76,7 @@ class DTPreprocessor {
 
 public:
   yaml_node_t *get(yaml_node_t *node, std::string const &name) {
-    yaml_node_pair_t *entry;
+    yaml_node_pair_t *entry = nullptr;
     for (entry = node->data.mapping.pairs.start;
          entry < node->data.mapping.pairs.top; ++entry) {
       yaml_node_t *key = yaml_document_get_node(doc, entry->key);
@@ -97,7 +97,7 @@ class DTPreprocessor {
 
   std::vector<std::string> vec(yaml_node_t *node) {
     std::vector<std::string> result;
-    yaml_node_item_t *entry;
+    yaml_node_item_t *entry = nullptr;
     for (entry = node->data.sequence.items.start;
          entry < node->data.sequence.items.top; ++entry) {
       result.push_back(str(yaml_document_get_node(doc, *entry)));
@@ -260,7 +260,7 @@ class DTPreprocessor {
          iter < list->data.sequence.items.top; ++iter) {
       auto *_case = yaml_document_get_node(doc, *iter);
       std::vector<std::pair<std::string, llvm::Type *>> bindings;
-      KORESymbol *symbol;
+      KORESymbol *symbol = nullptr;
       if (kind == SwitchLiteral || kind == CheckNull) {
         symbol = dv;
       } else {