Decide on and enforce house C++ style

Baltoli commented 8 months ago

Introduction

After https://github.com/runtimeverification/llvm-backend/pull/992 [^1], I think it's worth putting the effort in to properly clean up our C++ code with a house style. We use a bunch of different styles in the C++ code, and there is no strong consensus for what the correct solution will look like. This issue is an attempt at an opinionated set of defaults that we can bikeshed, then start to apply gradually to the code.[^2]

[!NOTE] There are a few things that are out of scope here. We already have a consistent structural style for the code that's enforced conveniently by clang-format, and similarly we have a lot of semantic best practices and conventions being enforced by clang-tidy. We can therefore restrict the bikeshedding here to naming aesthetics and consistency.

Proposed Style


#define MACRO_IF_NEEDED(x) x

namespace some_namespace_name {

enum class some_enum {
  variant_a,
  variant_b,
};

template <typename TypeParameter>
class some_class_name {
public:
  void member_function() const;

private:
  int member_variable_ = 0;
};

void some_class_name::member_function() const {
  auto local_var = f();
  free_function(member_variable_);
}

void free_function(int my_parameter_name) {
  ...
}

}

Some things we should also consider that are not directly naming style:

Prefixing extern "C" functions with a prefix like kllvm_ (see #992)
Using namespaces rather than purely textual namespacing (e.g. kore::composite_pattern rather than kllvm::KORECompositePattern).

Migration Strategy

Once we agree on a style, it should be easily achievable to migrate one stylistic element at a time using clang-tidy.

[^1]: We'd have avoided this pain by enforcing a consistent naming prefix for our C code so as to remain hygienic when interfacing with the outside world. Nobody other than us is naming their functions kllvm_arena_alloc! [^2]: We can hopefully clean up or improve some of the bigger, gnarlier functions that are currently exempted from clang-tidy's congitive-complexity warnings when we do this.

ehildenb commented 8 months ago

All for it! I think it's best to just pick a style, make the PR, and then let people nit at it. But I think general consensus is that a consistent style is more important than any particular choice.

Baltoli commented 8 months ago

Indeed - it's blocked on #995 but once that's resolved I can use the tooling to just apply this style globally and see if there are nitpicks

runtimeverification / llvm-backend