Perform two pass semantic analysis to check types of expressions

src/ast.cpp

 #include "ast.h"
+#include "parser.tab.hh"
 
 namespace ebpf {
 namespace bpftrace {
@@ -52,6 +53,38 @@ void Program::accept(Visitor &v) {
   v.visit(*this);
 }
 
+std::string opstr(Binop &binop)
+{
+  switch (binop.op) {
+    case ebpf::bpftrace::Parser::token::EQ:    return "==";
+    case ebpf::bpftrace::Parser::token::NE:    return "!=";
+    case ebpf::bpftrace::Parser::token::LE:    return "<=";
+    case ebpf::bpftrace::Parser::token::GE:    return ">=";
+    case ebpf::bpftrace::Parser::token::LT:    return "<";
+    case ebpf::bpftrace::Parser::token::GT:    return ">";
+    case ebpf::bpftrace::Parser::token::LAND:  return "&&";
+    case ebpf::bpftrace::Parser::token::LOR:   return "||";
+    case ebpf::bpftrace::Parser::token::PLUS:  return "+";
+    case ebpf::bpftrace::Parser::token::MINUS: return "-";
+    case ebpf::bpftrace::Parser::token::MUL:   return "*";
+    case ebpf::bpftrace::Parser::token::DIV:   return "/";
+    case ebpf::bpftrace::Parser::token::MOD:   return "%";
+    case ebpf::bpftrace::Parser::token::BAND:  return "&";
+    case ebpf::bpftrace::Parser::token::BOR:   return "|";
+    case ebpf::bpftrace::Parser::token::BXOR:  return "^";
+    default: abort();
+  }
+}
+
+std::string opstr(Unop &unop)
+{
+  switch (unop.op) {
+    case ebpf::bpftrace::Parser::token::LNOT: return "!";
+    case ebpf::bpftrace::Parser::token::BNOT: return "~";
+    default: abort();
+  }
+}
+
 } // namespace ast
 } // namespace bpftrace
 } // namespace ebpf

src/ast.h

@@ -153,6 +153,9 @@ public:
   virtual void visit(Program &program) = 0;
 };
 
+std::string opstr(Binop &binop);
+std::string opstr(Unop &unop);
+
 } // namespace ast
 } // namespace bpftrace
 } // namespace ebpf

src/printer.cpp

 #include "printer.h"
 #include "ast.h"
-#include "parser.tab.hh"
 
 namespace ebpf {
 namespace bpftrace {
@@ -49,28 +48,7 @@ void Printer::visit(Map &map)
 void Printer::visit(Binop &binop)
 {
   std::string indent(depth_, ' ');
-  std::string opstr;
-  switch (binop.op) {
-    case ebpf::bpftrace::Parser::token::EQ:    opstr = "=="; break;
-    case ebpf::bpftrace::Parser::token::NE:    opstr = "!="; break;
-    case ebpf::bpftrace::Parser::token::LE:    opstr = "<="; break;
-    case ebpf::bpftrace::Parser::token::GE:    opstr = ">="; break;
-    case ebpf::bpftrace::Parser::token::LT:    opstr = "<"; break;
-    case ebpf::bpftrace::Parser::token::GT:    opstr = ">"; break;
-    case ebpf::bpftrace::Parser::token::LAND:  opstr = "&&"; break;
-    case ebpf::bpftrace::Parser::token::LOR:   opstr = "||"; break;
-    case ebpf::bpftrace::Parser::token::PLUS:  opstr = "+"; break;
-    case ebpf::bpftrace::Parser::token::MINUS: opstr = "-"; break;
-    case ebpf::bpftrace::Parser::token::MUL:   opstr = "*"; break;
-    case ebpf::bpftrace::Parser::token::DIV:   opstr = "/"; break;
-    case ebpf::bpftrace::Parser::token::MOD:   opstr = "%"; break;
-    case ebpf::bpftrace::Parser::token::BAND:  opstr = "&"; break;
-    case ebpf::bpftrace::Parser::token::BOR:   opstr = "|"; break;
-    case ebpf::bpftrace::Parser::token::BXOR:  opstr = "^"; break;
-    default: abort();
-  }
-
-  out_ << indent << opstr << std::endl;
+  out_ << indent << opstr(binop) << std::endl;
 
   ++depth_;
   binop.left->accept(*this);
@@ -81,14 +59,7 @@ void Printer::visit(Binop &binop)
 void Printer::visit(Unop &unop)
 {
   std::string indent(depth_, ' ');
-  std::string opstr;
-  switch (unop.op) {
-    case ebpf::bpftrace::Parser::token::LNOT: opstr = "!"; break;
-    case ebpf::bpftrace::Parser::token::BNOT: opstr = "~"; break;
-    default: abort();
-  }
-
-  out_ << indent << opstr << std::endl;
+  out_ << indent << opstr(unop) << std::endl;
 
   ++depth_;
   unop.expr->accept(*this);

src/semantic_analyser.cpp

@@ -28,16 +28,28 @@ void SemanticAnalyser::visit(Builtin &builtin)
 
 void SemanticAnalyser::visit(Call &call)
 {
+  int nargs = 0;
   if (call.vargs) {
+    nargs = call.vargs->size();
     for (Expression *expr : *call.vargs) {
       expr->accept(*this);
     }
   }
 
-  if (call.func == "quantize")
+  if (call.func == "quantize") {
     type_ = Type::quantize;
-  else if (call.func == "count")
+    if (nargs != 1) {
+      err_ << "quantize() should take 1 argument (";
+      err_ << nargs << " provided)" << std::endl;
+    }
+  }
+  else if (call.func == "count") {
     type_ = Type::count;
+    if (nargs != 0) {
+      err_ << "count() should take 0 arguments (";
+      err_ << nargs << " provided)" << std::endl;
+    }
+  }
   else {
     type_ = Type::none;
     err_ << "Unknown function: '" << call.func << "'" << std::endl;
@@ -60,50 +72,39 @@ void SemanticAnalyser::visit(Map &map)
       err_ << "Argument mismatch for " << map.ident << ": ";
       err_ << "trying to access with arguments: [ ";
       for (Type t : args) { err_ << typestr(t) << " "; }
-      err_ << "]" << std::endl;
-      err_ << "when map already uses the arguments: [ ";
+      err_ << "]\n\twhen map already uses the arguments: [ ";
       for (Type t : search->second) { err_ << typestr(t) << " "; }
-      err_ << "]" << std::endl;
+      err_ << "]\n" << std::endl;
     }
   }
   else {
     map_args_.insert({map.ident, args});
   }
+
+  type_ = map_val_.find(map.ident)->second;
 }
 
 void SemanticAnalyser::visit(Binop &binop)
 {
+  Type lhs, rhs;
   binop.left->accept(*this);
-  switch (binop.op) {
-    case ebpf::bpftrace::Parser::token::EQ:    break;
-    case ebpf::bpftrace::Parser::token::NE:    break;
-    case ebpf::bpftrace::Parser::token::LE:    break;
-    case ebpf::bpftrace::Parser::token::GE:    break;
-    case ebpf::bpftrace::Parser::token::LT:    break;
-    case ebpf::bpftrace::Parser::token::GT:    break;
-    case ebpf::bpftrace::Parser::token::LAND:  break;
-    case ebpf::bpftrace::Parser::token::LOR:   break;
-    case ebpf::bpftrace::Parser::token::PLUS:  break;
-    case ebpf::bpftrace::Parser::token::MINUS: break;
-    case ebpf::bpftrace::Parser::token::MUL:   break;
-    case ebpf::bpftrace::Parser::token::DIV:   break;
-    case ebpf::bpftrace::Parser::token::MOD:   break;
-    case ebpf::bpftrace::Parser::token::BAND:  break;
-    case ebpf::bpftrace::Parser::token::BOR:   break;
-    case ebpf::bpftrace::Parser::token::BXOR:  break;
-    default: abort();
-  }
+  lhs = type_;
   binop.right->accept(*this);
+  rhs = type_;
+
+  if (pass_ == 2 && lhs != rhs) {
+    err_ << "Type mismatch for '" << opstr(binop) << "': ";
+    err_ << "comparing '" << typestr(lhs) << "' ";
+    err_ << "with '" << typestr(rhs) << "'" << std::endl;
+  }
+
+  type_ = Type::integer;
 }
 
 void SemanticAnalyser::visit(Unop &unop)
 {
-  switch (unop.op) {
-    case ebpf::bpftrace::Parser::token::LNOT: break;
-    case ebpf::bpftrace::Parser::token::BNOT: break;
-    default: abort();
-  }
   unop.expr->accept(*this);
+  type_ = Type::integer;
 }
 
 void SemanticAnalyser::visit(ExprStatement &expr)
@@ -122,8 +123,8 @@ void SemanticAnalyser::visit(AssignMapStatement &assignment)
     if (search->second != type_) {
       err_ << "Type mismatch for " << map_ident << ": ";
       err_ << "trying to assign variable of type '" << typestr(type_);
-      err_ << "' when map already contains a '";
-      err_ << typestr(search->second) << "'" << std::endl;
+      err_ << "'\n\twhen map already contains a value of type '";
+      err_ << typestr(search->second) << "'\n" << std::endl;
     }
   }
   else {
@@ -143,9 +144,8 @@ void SemanticAnalyser::visit(AssignMapCallStatement &assignment)
     if (search->second != type_) {
       err_ << "Type mismatch for " << map_ident << ": ";
       err_ << "trying to assign result of '" << assignment.call->func;
-      err_ << "'" << typestr(type_);
-      err_ << "' when map already contains a '";
-      err_ << typestr(search->second) << "'" << std::endl;
+      err_ << "()'\n\twhen map already contains a value of type '";
+      err_ << typestr(search->second) << "'\n" << std::endl;
     }
   }
   else {
@@ -178,16 +178,21 @@ void SemanticAnalyser::visit(Program &program)
 
 int SemanticAnalyser::analyse()
 {
-  root_->accept(*this);
-
-  std::string errors = err_.str();
-  if (errors.empty()) {
-    return 0;
-  }
-  else {
-    std::cerr << errors;
-    return 1;
+  // Two pass analysis, to handle variables being used before they are defined:
+  // - First pass checks assignments
+  // - Second pass checks expressions
+  std::string errors;
+
+  for (pass_ = 1; pass_ <= 2; pass_++) {
+    root_->accept(*this);
+    errors = err_.str();
+    if (!errors.empty()) {
+      std::cerr << errors;
+      return pass_;
+    }
   }
+
+  return 0;
 }
 
 std::string SemanticAnalyser::typestr(Type t)

src/semantic_analyser.h

@@ -32,6 +32,7 @@ public:
 private:
   Node *root_;
   std::ostringstream err_;
+  int pass_;
 
   enum class Type
   {