Finish up the basic 'def' rewrite rules

include/serene/exprs/def.h

@@ -53,8 +53,8 @@ public:
   maybe_node analyze(reader::SemanticContext &);
 
   static bool classof(const Expression *e);
-  static std::shared_ptr<errors::Error> isValid(List *);
-
+  // static std::shared_ptr<errors::Error> isValid(List *);
+  static maybe_node make(List *);
   ~Def() = default;
 };
 

include/serene/exprs/expression.h

@@ -75,7 +75,13 @@ public:
   /// The AST representation of an expression
   virtual std::string toString() const = 0;
 
-  virtual maybe_node analyze(reader::SemanticContext &) = 0;
+  /// Analyzes the semantics of current node and return a new node in case
+  /// that we need to semantically rewrite the current node and replace it with
+  /// another node. For example to change from a List containing `(def a b)`
+  /// to a `Def` node that represents defining a new binding.
+  ///
+  /// \param ctx is the context object of the semantic analyzer.
+  virtual maybe_node analyze(reader::SemanticContext &ctx) = 0;
 };
 
 /// Create a new `node` of type `T` and forwards any given parameter
@@ -107,7 +113,11 @@ std::shared_ptr<T> makeAndCast(Args &&...args) {
   return std::make_shared<T>(std::forward<Args>(args)...);
 };
 
+/// Convert the given AST to string by calling the `toString` method
+/// of each node.
 std::string toString(ast &);
+
+/// Converts the given AST to string and prints it out
 void dump(ast &);
 
 } // namespace exprs

include/serene/reader/reader.h

@@ -95,6 +95,11 @@ public:
 
   ~FileReader();
 };
+
+/// Parses the given `input` string and returns a `Result<ast>`
+/// which may contains an AST or an `llvm::Error`
+exprs::maybe_ast read(llvm::StringRef input);
+
 } // namespace reader
 } // namespace serene
 #endif

src/serene/exprs/def.cpp

@@ -46,22 +46,23 @@ bool Def::classof(const Expression *e) {
   return e->getType() == ExprType::Def;
 };
 
-std::shared_ptr<errors::Error> Def::isValid(List *list) {
+maybe_node Def::make(List *list) {
   // TODO: Add support for docstring as the 3rd argument (4th element)
   if (list->count() != 3) {
-    std::string msg = llvm::formatv("Expected 3 got {}", list->count());
-    return makeAndCast<errors::Error>(&errors::DefWrongNumberOfArgs,
-                                      list->elements[0], msg);
+    std::string msg = llvm::formatv("Expected 3 got {0}", list->count());
+    return Result<node>::success(makeAndCast<errors::Error>(
+        &errors::DefWrongNumberOfArgs, list->elements[0], msg));
   }
 
   Symbol *binding = llvm::dyn_cast<Symbol>(list->elements[1].get());
 
   if (!binding) {
-    return makeAndCast<errors::Error>(&errors::DefExpectSymbol,
-                                      list->elements[1], "");
+    return Result<node>::success(makeAndCast<errors::Error>(
+        &errors::DefExpectSymbol, list->elements[1], ""));
   }
 
-  return nullptr;
+  node def = exprs::make<Def>(list->location, binding->name, list->elements[2]);
+  return Result<node>::success(def);
 };
 } // namespace exprs
 } // namespace serene

src/serene/exprs/list.cpp

@@ -62,22 +62,27 @@ maybe_node List::analyze(reader::SemanticContext &ctx) {
 
       if (sym) {
         if (sym->name == "def") {
-          auto maybeErr = Def::isValid(this);
-
-          if (maybeErr) {
-            // Not a valid `def` form
-            return Result<node>::success(maybeErr);
-          }
-
-          Symbol *binding = llvm::dyn_cast<Symbol>(elements[1].get());
-
-          if (!binding) {
-            llvm_unreachable("Def::isValid should of catch this.");
-          }
-
-          node def = make<Def>(location, binding->name, elements[2]);
-          return Result<node>::success(def);
+          return Def::make(this);
         }
+
+        /* if (sym->name == "fn") { */
+        /*   auto maybeErr = Fn::isValid(this); */
+
+        /*   if (maybeErr) { */
+        /*     // Not a valid `def` form */
+        /*     return Result<node>::success(maybeErr); */
+        /*   } */
+
+        /*   Symbol *binding = llvm::dyn_cast<Symbol>(elements[1].get()); */
+
+        /*   if (!binding) { */
+        /*     llvm_unreachable("Def::isValid should of catch this."); */
+        /*   } */
+
+        /*   node def = make<Def>(location, binding->name, elements[2]); */
+        /*   return Result<node>::success(def); */
+
+        /* } */
       }
 
     }

src/serene/reader/reader.cpp

@@ -332,5 +332,11 @@ FileReader::~FileReader() {
   delete this->reader;
   READER_LOG("Destroying the file reader");
 }
+
+exprs::maybe_ast read(llvm::StringRef input) {
+  reader::Reader r(input);
+  auto ast = r.read();
+  return ast;
+}
 } // namespace reader
 } // namespace serene

src/serene/reader/semantics.cpp

@@ -26,6 +26,16 @@
 #include "serene/exprs/expression.h"
 
 namespace serene::reader {
+
+/// The entry point to the Semantic analysis phase. It calls the `analyze`
+/// method of each node in the given AST and creates a new AST that contains a
+/// more comprehensive set of nodes in a semantically correct AST. If the
+/// `analyze` method of a node return a `nullptr` value as the `success` result
+/// (Checkout the `Result` type in `utils.h`) then the original node will be
+/// used instead. Also please note that in **Serene** Semantic errors
+/// represented as AST nodes as well. So you should expect an `analyze` method
+/// of a node to return a `Result<node>::Success(Error...)` in case of a
+/// semantic error.
 exprs::maybe_ast Semantics::analyze(exprs::ast &inputAst) {
   // TODO: Fetch the current namespace from the JIT engine later and if it is
   // `nil` then the given `ast` has to start with a namespace definition.
@@ -35,18 +45,26 @@ exprs::maybe_ast Semantics::analyze(exprs::ast &inputAst) {
   for (auto &element : inputAst) {
     auto maybeNode = element->analyze(context);
 
+    // Is it a `success` result
     if (maybeNode) {
       auto &node = maybeNode.getValue();
 
       if (node) {
+        // is there a new node to replace the current node ?
         ast.push_back(node);
       } else {
+        // Analyze returned a `nullptr`. No rewrite is needed.
+        // Use the current element instead.
         ast.push_back(element);
       }
     } else {
+
+      // `analyze` returned an errorful result. This type of error
+      // is llvm related and has to be raised later
       Result<exprs::ast>::error(std::move(maybeNode.getError()));
     }
   }
+
   return Result<exprs::ast>::success(std::move(ast));
 };
 }; // namespace serene::reader

src/tests/exprs/list_tests.cpp.inc

@@ -81,16 +81,26 @@ TEST_CASE("List Expression", "[expression]") {
 
   TEST_CASE("List semantic analysis", "[semantic]") {
     reader::Semantics analyzer;
-    auto r = new reader::Reader("(def (a b) 4)");
-    auto ast = r->read().getValue();
-
-    //auto &ast = maybeAst.getValue();
-    auto afterAst = analyzer.analyze(ast);
-
-
+    auto ast = reader::read("(def (a) b)");
+    auto afterAst = analyzer.analyze(ast.getValue());
     REQUIRE(afterAst);
     CHECK(toString(afterAst.getValue()) == "<Error E1: >");
-    delete r;
+
+    ast = reader::read("(def a)");
+    afterAst = analyzer.analyze(ast.getValue());
+    REQUIRE(afterAst);
+    CHECK(toString(afterAst.getValue()) == "<Error E2: Expected 3 got 2>");
+
+    ast = reader::read("(def a b c)");
+    afterAst = analyzer.analyze(ast.getValue());
+    REQUIRE(afterAst);
+    CHECK(toString(afterAst.getValue()) == "<Error E2: Expected 3 got 4>");
+
+    ast = reader::read("(def a b)");
+    afterAst = analyzer.analyze(ast.getValue());
+    REQUIRE(afterAst);
+    CHECK(toString(afterAst.getValue()) == "<Def a -> <Symbol b>>");
+
   }
 } // namespace exprs
 } // namespace serene