Enable clang-tidy readability checks (#936)

This is another PR in my ongoing series to enable more clang-tidy checks
for the codebase. As previously, each commit in this PR fixes the issues
flagged by a single check, and can / should be reviewed independently.

As ever, I'm happy to bikeshed any of the specific changes in this PR.
Note that for
[`readability-function-cognitive-complexity`](c2b0de1),
I've not actually refactored any existing code in the interest of
keeping this diff minimal, but the check will prevent us from
introducing new code that exceeds the complexity threshold.

.clang-tidy

@@ -9,6 +9,11 @@ Checks:
   -modernize-use-trailing-return-type
   performance-*
   -performance-no-int-to-ptr
+  readability-*
+  -readability-magic-numbers
+  -readability-identifier-length
+  -readability-implicit-bool-conversion
+  -readability-named-parameter
   bugprone-*
   -bugprone-easily-swappable-parameters
   -bugprone-narrowing-conversions
@@ -22,3 +27,5 @@ WarningsAsErrors:
 CheckOptions:
   - key: 'misc-const-correctness.AnalyzeValues'
     value: false
+  - key: 'readability-function-cognitive-complexity.IgnoreMacros'
+    value: true

bindings/c/lib.cpp

@@ -49,7 +49,7 @@ std::optional<pretty_print_definition> get_print_data();
  */
 
 extern "C" {
-block *take_steps(int64_t, block *);
+
 void initStaticObjects(void);
 void freeAllKoreMem(void);
 }
@@ -124,7 +124,7 @@ char *kore_pattern_pretty_print(kore_pattern const *pat) {
   fs::remove_all(temp_dir_name);
 
   auto pretty_str = ss.str();
-  auto data
+  auto *data
       = static_cast<char *>(malloc(sizeof(char) * (pretty_str.size() + 1)));
 
   std::copy(pretty_str.begin(), pretty_str.end(), data);
@@ -152,69 +152,69 @@ void kore_pattern_free(kore_pattern const *pat) {
 }
 
 kore_pattern *kore_pattern_parse(char const *kore_text) {
-  auto pat = new kore_pattern;
+  auto *pat = new kore_pattern;
   pat->ptr_ = kllvm::parser::KOREParser::from_string(kore_text)->pattern();
   return pat;
 }
 
 kore_pattern *kore_pattern_parse_file(char const *filename) {
-  auto pat = new kore_pattern;
+  auto *pat = new kore_pattern;
   pat->ptr_ = kllvm::parser::KOREParser(std::string(filename)).pattern();
   return pat;
 }
 
 kore_pattern *kore_pattern_new_token(char const *value, kore_sort const *sort) {
-  auto pat = kore_string_pattern_new(value);
-  auto ret = kore_pattern_new_token_internal(pat, sort);
+  auto *pat = kore_string_pattern_new(value);
+  auto *ret = kore_pattern_new_token_internal(pat, sort);
 
   kore_pattern_free(pat);
   return ret;
 }
 
 kore_pattern *kore_pattern_new_token_with_len(
     char const *value, size_t len, kore_sort const *sort) {
-  auto pat = kore_string_pattern_new_with_len(value, len);
-  auto ret = kore_pattern_new_token_internal(pat, sort);
+  auto *pat = kore_string_pattern_new_with_len(value, len);
+  auto *ret = kore_pattern_new_token_internal(pat, sort);
 
   kore_pattern_free(pat);
   return ret;
 }
 
 kore_pattern *kore_pattern_new_injection(
     kore_pattern const *term, kore_sort const *from, kore_sort const *to) {
-  auto inj = new kore_pattern;
+  auto *inj = new kore_pattern;
   inj->ptr_ = kllvm::bindings::make_injection(term->ptr_, from->ptr_, to->ptr_);
   return inj;
 }
 
 kore_pattern *kore_pattern_make_interpreter_input(
     kore_pattern const *pgm, kore_sort const *sort) {
-  auto config_sort = kore_composite_sort_new("SortKConfigVar");
-  auto kitem_sort = kore_composite_sort_new("SortKItem");
+  auto *config_sort = kore_composite_sort_new("SortKConfigVar");
+  auto *kitem_sort = kore_composite_sort_new("SortKItem");
 
-  auto pgm_token = kore_pattern_new_token("$PGM", config_sort);
-  auto key = kore_pattern_new_injection(pgm_token, config_sort, kitem_sort);
+  auto *pgm_token = kore_pattern_new_token("$PGM", config_sort);
+  auto *key = kore_pattern_new_injection(pgm_token, config_sort, kitem_sort);
   kore_pattern_free(pgm_token);
 
-  auto map_item = kore_composite_pattern_new("Lbl'UndsPipe'-'-GT-Unds'");
+  auto *map_item = kore_composite_pattern_new("Lbl'UndsPipe'-'-GT-Unds'");
   kore_composite_pattern_add_argument(map_item, key);
   kore_pattern_free(key);
 
   if (kore_sort_is_kitem(sort)) {
     kore_composite_pattern_add_argument(map_item, pgm);
   } else {
-    auto inj = kore_pattern_new_injection(pgm, sort, kitem_sort);
+    auto *inj = kore_pattern_new_injection(pgm, sort, kitem_sort);
     kore_composite_pattern_add_argument(map_item, inj);
     kore_pattern_free(inj);
   }
 
-  auto map_unit = kore_composite_pattern_new("Lbl'Stop'Map");
+  auto *map_unit = kore_composite_pattern_new("Lbl'Stop'Map");
 
-  auto map_concat = kore_composite_pattern_new("Lbl'Unds'Map'Unds'");
+  auto *map_concat = kore_composite_pattern_new("Lbl'Unds'Map'Unds'");
   kore_composite_pattern_add_argument(map_concat, map_unit);
   kore_composite_pattern_add_argument(map_concat, map_item);
 
-  auto top_cell = kore_composite_pattern_new("LblinitGeneratedTopCell");
+  auto *top_cell = kore_composite_pattern_new("LblinitGeneratedTopCell");
   kore_composite_pattern_add_argument(top_cell, map_concat);
 
   kore_sort_free(config_sort);
@@ -227,7 +227,7 @@ kore_pattern *kore_pattern_make_interpreter_input(
 }
 
 kore_pattern *kore_pattern_desugar_associative(kore_pattern const *pat) {
-  auto ret = new kore_pattern;
+  auto *ret = new kore_pattern;
   ret->ptr_ = pat->ptr_->desugarAssociative();
   return ret;
 }
@@ -237,18 +237,18 @@ block *kore_pattern_construct(kore_pattern const *pat) {
 }
 
 char *kore_block_dump(block *term) {
-  auto hooked_str = printConfigurationToString(term)->data;
+  auto *hooked_str = printConfigurationToString(term)->data;
   auto len = std::strlen(hooked_str);
 
-  auto new_str = static_cast<char *>(malloc((len + 1) * sizeof(char)));
+  auto *new_str = static_cast<char *>(malloc((len + 1) * sizeof(char)));
   std::strncpy(new_str, hooked_str, len);
   new_str[len] = '\0';
 
   return new_str;
 }
 
 kore_pattern *kore_pattern_from_block(block *term) {
-  auto pat = new kore_pattern;
+  auto *pat = new kore_pattern;
   pat->ptr_ = kllvm::bindings::term_to_pattern(term);
   return pat;
 }
@@ -264,16 +264,16 @@ bool kore_simplify_bool(kore_pattern const *pattern) {
 void kore_simplify(
     kore_pattern const *pattern, kore_sort const *sort, char **data_out,
     size_t *size_out) {
-  auto block = kllvm::bindings::simplify_to_term(pattern->ptr_, sort->ptr_);
+  auto *block = kllvm::bindings::simplify_to_term(pattern->ptr_, sort->ptr_);
   serializeConfiguration(block, "SortKItem{}", data_out, size_out, true);
 }
 
 void kore_simplify_binary(
     char *data_in, size_t size_in, kore_sort const *sort, char **data_out,
     size_t *size_out) {
-  auto sort_str = kore_sort_dump(sort);
+  auto *sort_str = kore_sort_dump(sort);
 
-  auto block = deserializeConfiguration(data_in, size_in);
+  auto *block = deserializeConfiguration(data_in, size_in);
   serializeConfiguration(block, sort_str, data_out, size_out, true);
 
   free(sort_str);
@@ -282,20 +282,20 @@ void kore_simplify_binary(
 /* KORECompositePattern */
 
 kore_pattern *kore_composite_pattern_new(char const *name) {
-  auto pat = new kore_pattern;
+  auto *pat = new kore_pattern;
   pat->ptr_ = kllvm::KORECompositePattern::Create(std::string(name));
   return pat;
 }
 
 kore_pattern *kore_composite_pattern_from_symbol(kore_symbol *sym) {
-  auto pat = new kore_pattern;
+  auto *pat = new kore_pattern;
   pat->ptr_ = kllvm::KORECompositePattern::Create(sym->ptr_.get());
   return pat;
 }
 
 void kore_composite_pattern_add_argument(
     kore_pattern *pat, kore_pattern const *arg) {
-  if (auto cast_ptr
+  if (auto *cast_ptr
       = dynamic_cast<kllvm::KORECompositePattern *>(pat->ptr_.get())) {
     cast_ptr->addArgument(arg->ptr_);
   } else {
@@ -339,14 +339,14 @@ bool kore_sort_is_k(kore_sort const *sort) {
 /* KORECompositeSort */
 
 kore_sort *kore_composite_sort_new(char const *name) {
-  auto sort = new kore_sort;
+  auto *sort = new kore_sort;
   sort->ptr_ = kllvm::KORECompositeSort::Create(std::string(name));
   return sort;
 }
 
 void kore_composite_sort_add_argument(
     kore_sort const *sort, kore_sort const *arg) {
-  if (auto cast_ptr
+  if (auto *cast_ptr
       = dynamic_cast<kllvm::KORECompositeSort *>(sort->ptr_.get())) {
     cast_ptr->addArgument(arg->ptr_);
   } else {
@@ -357,7 +357,7 @@ void kore_composite_sort_add_argument(
 /* KORESymbol */
 
 kore_symbol *kore_symbol_new(char const *name) {
-  auto sym = new kore_symbol;
+  auto *sym = new kore_symbol;
   sym->ptr_ = kllvm::KORESymbol::Create(std::string(name));
   return sym;
 }
@@ -391,24 +391,24 @@ char *get_c_string(std::string const &str) {
   // Include null terminator
   auto total_length = str.length() + 1;
 
-  auto c_str = reinterpret_cast<char *>(malloc(total_length * sizeof(char)));
+  auto *c_str = reinterpret_cast<char *>(malloc(total_length * sizeof(char)));
   std::strncpy(c_str, str.c_str(), total_length);
 
   return c_str;
 }
 
 kore_pattern *kore_string_pattern_new_internal(std::string const &str) {
-  auto pat = new kore_pattern;
+  auto *pat = new kore_pattern;
   pat->ptr_ = kllvm::KOREStringPattern::Create(str);
   return pat;
 }
 
 kore_pattern *kore_pattern_new_token_internal(
     kore_pattern const *value, kore_sort const *sort) {
-  auto sym = kore_symbol_new("\\dv");
+  auto *sym = kore_symbol_new("\\dv");
   kore_symbol_add_formal_argument(sym, sort);
 
-  auto pat = kore_composite_pattern_from_symbol(sym);
+  auto *pat = kore_composite_pattern_from_symbol(sym);
   kore_composite_pattern_add_argument(pat, value);
 
   kore_symbol_free(sym);

bindings/core/src/core.cpp

@@ -74,10 +74,9 @@ block *simplify_to_term(
 
   if (is_sort_kitem(sort) || is_sort_k(sort)) {
     return construct_term(pattern);
-  } else {
-    auto rawTerm = make_rawTerm(pattern, sort, kitem_sort);
-    return construct_term(rawTerm);
   }
+  auto rawTerm = make_rawTerm(pattern, sort, kitem_sort);
+  return construct_term(rawTerm);
 }
 
 std::shared_ptr<KOREPattern> simplify(
@@ -95,8 +94,8 @@ evaluate_function(std::shared_ptr<KORECompositePattern> const &term) {
 
   auto label = ast_to_string(*term->getConstructor());
   auto tag = getTagForSymbolName(label.c_str());
-  auto return_sort = getReturnSortForTag(tag);
-  auto result = evaluateFunctionSymbol(tag, term_args.data());
+  const auto *return_sort = getReturnSortForTag(tag);
+  auto *result = evaluateFunctionSymbol(tag, term_args.data());
 
   return sortedTermToKorePattern(static_cast<block *>(result), return_sort);
 }

bindings/python/runtime.cpp

@@ -73,7 +73,7 @@ void bind_runtime(py::module_ &m) {
       .def(
           "__str__",
           [](block *term) {
-            auto k_str = printConfigurationToString(term);
+            auto *k_str = printConfigurationToString(term);
             return std::string(k_str->data, len(k_str));
           })
       .def("step", [](block *term, int64_t n) { return take_steps(n, term); })

include/kllvm/ast/AST.h

@@ -163,8 +163,8 @@ class KORECompositeSort : public KORESort {
 
   const std::string getName() const { return name; }
   ValueType getCategory(KOREDefinition *definition);
-  std::string getHook(KOREDefinition *definition);
-  static ValueType getCategory(std::string const &hook);
+  std::string getHook(KOREDefinition *definition) const;
+  static ValueType getCategory(std::string const &hookName);
 
   virtual bool isConcrete() const override;
   virtual sptr<KORESort> substitute(const substitution &subst) override;

include/kllvm/binary/serializer.h

@@ -145,7 +145,7 @@ class serializer {
    * Calculate the number of continued bytes required to serialize this value as
    * a length field, without actually doing so.
    */
-  int required_chunks(uint64_t len) const;
+  static int required_chunks(uint64_t len);
 };
 
 template <typename It>

include/kllvm/codegen/Decision.h

@@ -387,7 +387,7 @@ class Decision {
 
   llvm::AllocaInst *decl(var_type const &name);
 
-  llvm::Constant *stringLiteral(std::string const &name);
+  llvm::Constant *stringLiteral(std::string const &str);
   llvm::Value *ptrTerm(llvm::Value *val);
 
 public:

include/kllvm/parser/KOREParser.h

@@ -31,7 +31,8 @@ class KOREParser {
 private:
   KOREScanner scanner;
   location loc;
-  [[noreturn]] void error(const location &loc, const std::string &err_message);
+  [[noreturn]] static void
+  error(const location &loc, const std::string &err_message);
 
   std::string consume(token next);
   token peek();

include/runtime/alloc.h

@@ -38,7 +38,7 @@ void *koreAllocAlwaysGC(size_t requested);
 // generation
 void koreAllocSwap(bool swapOld);
 // resizes the last allocation into the young generation
-void *koreResizeLastAlloc(void *oldptr, size_t newrequest, size_t oldrequest);
+void *koreResizeLastAlloc(void *oldptr, size_t newrequest, size_t last_size);
 // allocator hook for the GMP library
 void *koreAllocMP(size_t);
 // reallocator hook for the GMP library

include/runtime/header.h

@@ -26,14 +26,14 @@
 struct MatchLog {
   enum { SUCCESS = 0, FUNCTION, FAIL } kind;
 
-  char *function;
-  char *debugName;
+  char const *function;
+  char const *debugName;
   void *result;
   std::vector<void *> args;
 
-  char *pattern;
+  char const *pattern;
   void *subject;
-  char *sort;
+  char const *sort;
 };
 
 // the actual length is equal to the block header with the gc bits masked out.