Fix the exprs dir's tidy issues

include/serene/exprs/list.h

@@ -23,6 +23,7 @@
 #include "serene/exprs/expression.h"
 
 #include <llvm/ADT/Optional.h>
+
 #include <string>
 
 namespace serene {
@@ -51,7 +52,7 @@ public:
 
   size_t count() const;
 
-  Ast from(uint begin);
+  Ast from(uint index);
 
   llvm::Optional<Expression *> at(uint index);
 
@@ -71,7 +72,8 @@ public:
   /// by the for loop.
   std::vector<Node>::iterator end();
 
-  MaybeNode analyze(SereneContext &) override;
+  MaybeNode analyze(SereneContext &ctx) override;
+  // NOLINTNEXTLINE(readability-named-parameter)
   void generateIR(serene::Namespace &, mlir::ModuleOp &) override{};
 
   ~List() = default;

include/serene/exprs/number.h

@@ -51,7 +51,7 @@ struct Number : public Expression {
   void generateIR(serene::Namespace &, mlir::ModuleOp &) override;
 
   // TODO: This is horrible, we need to fix it after the mvp
-  int toI64();
+  int toI64() const;
 
   ~Number() = default;
 

include/serene/jit.h

@@ -34,9 +34,10 @@
 #include <llvm/ExecutionEngine/Orc/LLJIT.h>
 #include <llvm/Support/CodeGen.h>
 #include <llvm/Support/Debug.h>
-#include <memory>
 #include <mlir/Support/LLVM.h>
 
+#include <memory>
+
 #define JIT_LOG(...) \
   DEBUG_WITH_TYPE("JIT", llvm::dbgs() << "[JIT]: " << __VA_ARGS__ << "\n");
 
@@ -95,8 +96,9 @@ public:
 
   /// Invokes the function with the given name passing it the list of opaque
   /// pointers to the actual arguments.
-  llvm::Error invokePacked(llvm::StringRef name,
-                           llvm::MutableArrayRef<void *> args = llvm::None);
+  llvm::Error
+  invokePacked(llvm::StringRef name,
+               llvm::MutableArrayRef<void *> args = llvm::None) const;
 
   /// Trait that defines how a given type is passed to the JIT code. This
   /// defaults to passing the address but can be specialized.

src/libserene/exprs/call.cpp

@@ -86,7 +86,7 @@ MaybeNode Call::make(SereneContext &ctx, List *list) {
 
     auto *sym = llvm::dyn_cast<Symbol>(first.get());
 
-    if (!sym) {
+    if (sym == nullptr) {
       llvm_unreachable("Couldn't case to Symbol while the type is symbol!");
     }
 

src/libserene/exprs/def.cpp

@@ -65,7 +65,7 @@ MaybeNode Def::make(SereneContext &ctx, List *list) {
 
   // Make sure that the first argument is a Symbol
   Symbol *binding = llvm::dyn_cast<Symbol>(list->elements[1].get());
-  if (!binding) {
+  if (binding == nullptr) {
     return makeErrorful<Node>(list->elements[1]->location,
                               &errors::DefExpectSymbol, "");
   }
@@ -93,7 +93,7 @@ MaybeNode Def::make(SereneContext &ctx, List *list) {
 
   if (analyzedValue->getType() == ExprType::Fn) {
     Fn *tmp = llvm::dyn_cast<Fn>(analyzedValue.get());
-    if (!tmp) {
+    if (tmp == nullptr) {
       llvm_unreachable("inconsistent getType for function");
     }
 
@@ -107,9 +107,8 @@ MaybeNode Def::make(SereneContext &ctx, List *list) {
   if (result.succeeded()) {
     return makeSuccessfulNode<Def>(list->location, binding->name,
                                    analyzedValue);
-  } else {
-    llvm_unreachable("Inserting a value in the semantic env failed!");
   }
+  llvm_unreachable("Inserting a value in the semantic env failed!");
 };
 
 void Def::generateIR(serene::Namespace &ns, mlir::ModuleOp &m) {

src/libserene/exprs/expression.cpp

@@ -24,7 +24,7 @@ namespace serene {
 namespace exprs {
 
 std::string astToString(const Ast *tree) {
-  if (tree->size() == 0) {
+  if (tree->empty()) {
     return "";
   }
 

src/libserene/exprs/fn.cpp

@@ -26,17 +26,18 @@
 #include "serene/slir/dialect.h"
 #include "serene/slir/utils.h"
 
-#include <cstdint>
 #include <llvm/Support/Casting.h>
 #include <llvm/Support/FormatVariadic.h>
 #include <mlir/IR/Block.h>
 #include <mlir/IR/BuiltinAttributes.h>
 
+#include <utility>
+
 namespace serene {
 namespace exprs {
 
 Fn::Fn(SereneContext &ctx, reader::LocationRange &loc, List &args, Ast body)
-    : Expression(loc), args(args), body(body) {
+    : Expression(loc), args(args), body(std::move(body)) {
   this->setName(
       llvm::formatv("___fn___{0}", ctx.getCurrentNS().nextFnCounter()));
 };
@@ -57,7 +58,7 @@ MaybeNode Fn::analyze(SereneContext &ctx) {
 
 bool Fn::classof(const Expression *e) { return e->getType() == ExprType::Fn; };
 
-void Fn::setName(std::string n) { this->name = n; };
+void Fn::setName(std::string n) { this->name = std::move(n); };
 
 MaybeNode Fn::make(SereneContext &ctx, List *list) {
   // TODO: Add support for docstring as the 3rd argument (4th element)
@@ -75,7 +76,7 @@ MaybeNode Fn::make(SereneContext &ctx, List *list) {
 
   List *args = llvm::dyn_cast<List>(list->elements[1].get());
 
-  if (!args) {
+  if (args == nullptr) {
     std::string msg =
         llvm::formatv("Arguments of a function has to be a list, got '{0}'",
                       stringifyExprType(list->elements[1]->getType()));
@@ -114,7 +115,7 @@ void Fn::generateIR(serene::Namespace &ns, mlir::ModuleOp &m) {
   for (auto &arg : args) {
     auto *argSym = llvm::dyn_cast<Symbol>(arg.get());
 
-    if (!argSym) {
+    if (argSym == nullptr) {
       m->emitError(llvm::formatv(
           "Arguments of a function have to be symbols. Fn: '{0}'", name));
       return;

src/libserene/exprs/list.cpp

@@ -25,11 +25,13 @@
 #include "serene/exprs/fn.h"
 #include "serene/exprs/symbol.h"
 
-#include <iterator>
 #include <llvm/Support/Casting.h>
 #include <llvm/Support/ErrorHandling.h>
 #include <llvm/Support/FormatVariadic.h>
 
+#include <iterator>
+#include <utility>
+
 namespace serene {
 namespace exprs {
 
@@ -42,14 +44,14 @@ List::List(const reader::LocationRange &loc, Node &e) : Expression(loc) {
 };
 
 List::List(const reader::LocationRange &loc, Ast elems)
-    : Expression(loc), elements(elems){};
+    : Expression(loc), elements(std::move(elems)){};
 
 ExprType List::getType() const { return ExprType::List; };
 
 std::string List::toString() const {
   std::string s{this->elements.empty() ? "-" : ""};
 
-  for (auto &n : this->elements) {
+  for (const auto &n : this->elements) {
     s = llvm::formatv("{0} {1}", s, n->toString());
   }
 
@@ -63,7 +65,7 @@ MaybeNode List::analyze(SereneContext &ctx) {
     if (first->getType() == ExprType::Symbol) {
       auto *sym = llvm::dyn_cast<Symbol>(first);
 
-      if (sym) {
+      if (sym != nullptr) {
         if (sym->name == "def") {
           return Def::make(ctx, this);
         }

src/libserene/exprs/number.cpp

@@ -47,7 +47,7 @@ bool Number::classof(const Expression *e) {
   return e->getType() == ExprType::Number;
 };
 
-int Number::toI64() { return std::stoi(this->value); };
+int Number::toI64() const { return std::stoi(this->value); };
 
 void Number::generateIR(serene::Namespace &ns, mlir::ModuleOp &m) {
   mlir::OpBuilder builder(&ns.getContext().mlirContext);

src/libserene/jit.cpp

@@ -40,8 +40,13 @@
 #include <llvm/Support/ToolOutputFile.h>
 #include <mlir/ExecutionEngine/ExecutionEngine.h>
 #include <mlir/Support/FileUtilities.h>
+
 #include <stdexcept>
 
+#define COMMON_ARGS_COUNT 8
+// Just to make the linter happy
+#define I64_BIT_SIZE 64
+
 namespace serene {
 
 // TODO: Remove this function and replace it by our own version of
@@ -64,13 +69,13 @@ static void packFunctionArguments(llvm::Module *module) {
     if (func.isDeclaration()) {
       continue;
     }
-    if (interfaceFunctions.count(&func)) {
+    if (interfaceFunctions.count(&func) != 0) {
       continue;
     }
 
     // Given a function `foo(<...>)`, define the interface function
     // `mlir_foo(i8**)`.
-    auto newType = llvm::FunctionType::get(
+    auto *newType = llvm::FunctionType::get(
         builder.getVoidTy(), builder.getInt8PtrTy()->getPointerTo(),
         /*isVarArg=*/false);
     auto newName = makePackedFunctionName(func.getName());
@@ -81,15 +86,15 @@ static void packFunctionArguments(llvm::Module *module) {
 
     // Extract the arguments from the type-erased argument list and cast them to
     // the proper types.
-    auto bb = llvm::BasicBlock::Create(ctx);
+    auto *bb = llvm::BasicBlock::Create(ctx);
     bb->insertInto(interfaceFunc);
     builder.SetInsertPoint(bb);
     llvm::Value *argList = interfaceFunc->arg_begin();
-    llvm::SmallVector<llvm::Value *, 8> args;
+    llvm::SmallVector<llvm::Value *, COMMON_ARGS_COUNT> args;
     args.reserve(llvm::size(func.args()));
     for (auto &indexedArg : llvm::enumerate(func.args())) {
       llvm::Value *argIndex = llvm::Constant::getIntegerValue(
-          builder.getInt64Ty(), llvm::APInt(64, indexedArg.index()));
+          builder.getInt64Ty(), llvm::APInt(I64_BIT_SIZE, indexedArg.index()));
       llvm::Value *argPtrPtr =
           builder.CreateGEP(builder.getInt8PtrTy(), argList, argIndex);
       llvm::Value *argPtr =
@@ -106,7 +111,8 @@ static void packFunctionArguments(llvm::Module *module) {
     // Assuming the result is one value, potentially of type `void`.
     if (!result->getType()->isVoidTy()) {
       llvm::Value *retIndex = llvm::Constant::getIntegerValue(
-          builder.getInt64Ty(), llvm::APInt(64, llvm::size(func.args())));
+          builder.getInt64Ty(),
+          llvm::APInt(I64_BIT_SIZE, llvm::size(func.args())));
       llvm::Value *retPtrPtr =
           builder.CreateGEP(builder.getInt8PtrTy(), argList, retIndex);
       llvm::Value *retPtr =
@@ -213,10 +219,12 @@ MaybeJIT JIT::make(Namespace &ns,
         });
 
     // Register JIT event listeners if they are enabled.
-    if (jitEngine->gdbListener)
+    if (jitEngine->gdbListener != nullptr) {
       objectLayer->registerJITEventListener(*jitEngine->gdbListener);
-    if (jitEngine->perfListener)
+    }
+    if (jitEngine->perfListener != nullptr) {
       objectLayer->registerJITEventListener(*jitEngine->perfListener);
+    }
 
     // COFF format binaries (Windows) need special handling to deal with
     // exported symbol visibility.
@@ -256,11 +264,15 @@ MaybeJIT JIT::make(Namespace &ns,
   auto compileFunctionCreator = [&](llvm::orc::JITTargetMachineBuilder JTMB)
       -> llvm::Expected<
           std::unique_ptr<llvm::orc::IRCompileLayer::IRCompiler>> {
-    if (jitCodeGenOptLevel)
+    if (jitCodeGenOptLevel) {
       JTMB.setCodeGenOptLevel(jitCodeGenOptLevel.getValue());
+    }
+
     auto TM = JTMB.createTargetMachine();
-    if (!TM)
+    if (!TM) {
       return TM.takeError();
+    }
+
     return std::make_unique<llvm::orc::TMOwningSimpleCompiler>(
         std::move(*TM), jitEngine->cache.get());
   };
@@ -307,17 +319,22 @@ llvm::Expected<void (*)(void **)> JIT::lookup(llvm::StringRef name) const {
   }
 
   auto rawFPtr = expectedSymbol->getAddress();
-  auto fptr    = reinterpret_cast<void (*)(void **)>(rawFPtr);
-  if (!fptr)
+  // NOLINTNEXTLINE(performance-no-int-to-ptr)
+  auto fptr = reinterpret_cast<void (*)(void **)>(rawFPtr);
+
+  if (fptr == nullptr) {
     return make_string_error("looked up function is null");
+  }
+
   return fptr;
 }
 
 llvm::Error JIT::invokePacked(llvm::StringRef name,
-                              llvm::MutableArrayRef<void *> args) {
+                              llvm::MutableArrayRef<void *> args) const {
   auto expectedFPtr = lookup(name);
-  if (!expectedFPtr)
+  if (!expectedFPtr) {
     return expectedFPtr.takeError();
+  }
   auto fptr = *expectedFPtr;
 
   (*fptr)(args.data());