ScopingVisitor new error handling

codon/compiler/error.h

@@ -219,6 +219,11 @@ public:
   static char ID;
 };
 
+template <class... TA> std::string Eformat(const TA &...args) { return ""; }
+template <class... TA> std::string Eformat(const char *fmt, const TA &...args) {
+  return fmt::format(fmt, args...);
+}
+
 template <class... TA> std::string Emsg(Error e, const TA &...args) {
   switch (e) {
   /// Validations
@@ -440,8 +445,7 @@ template <class... TA> std::string Emsg(Error e, const TA &...args) {
     return fmt::format(
         "maximum realization depth reached during the realization of '{}'", args...);
   case Error::CUSTOM:
-    return fmt::format("{}", args...);
-
+    return Eformat(args...);
   default:
     assert(false);
   }

codon/parser/ast/error.h

@@ -51,6 +51,7 @@ public:
   int getErrorCode() const { return errorCode; }
   SrcInfo getSrcInfo() const { return loc; }
   void setSrcInfo(const SrcInfo &s) { loc = s; }
+  bool operator==(const ErrorMessage &t) const { return msg == t.msg && loc == t.loc; }
 
   void log(llvm::raw_ostream &out) const {
     if (!getFile().empty()) {
@@ -81,6 +82,7 @@ struct ParserErrors {
     void addMessage(const std::string &msg, const SrcInfo &info = SrcInfo()) {
       trace.emplace_back(msg, info);
     }
+    bool operator==(const Backtrace &t) const { return trace == t.trace; }
   };
   std::vector<Backtrace> errors;
 
@@ -101,17 +103,24 @@ struct ParserErrors {
   auto end() const { return errors.end(); }
   auto empty() const { return errors.empty(); }
   auto size() const { return errors.size(); }
+  auto &back() { return errors.back(); }
+  const auto &back() const { return errors.back(); }
   void append(const ParserErrors &e) {
-    errors.insert(errors.end(), e.errors.begin(), e.errors.end());
+    for (auto &trace : e)
+      addError(trace);
   }
 
-  Backtrace &getLast() {
+  Backtrace getLast() {
     assert(!empty() && "empty error trace");
     return errors.back();
   }
 
   /// Add an error message to the current backtrace
-  void addError(const std::vector<ErrorMessage> &trace) { errors.push_back({trace}); }
+  void addError(const Backtrace &trace) {
+    if (errors.empty() || !(errors.back() == trace))
+      errors.push_back({trace});
+  }
+  void addError(const std::vector<ErrorMessage> &trace) { addError(Backtrace{trace}); }
   std::string getMessage() const {
     if (empty())
       return "";
@@ -136,7 +145,8 @@ public:
       : std::runtime_error(errors.getMessage()), errors(errors) {}
   ParserException(llvm::Error &&e) noexcept;
 
-  ParserErrors getErrors() const { return errors; }
+  const ParserErrors &getErrors() const { return errors; }
+  ParserErrors &getErrors() { return errors; }
 };
 
 } // namespace codon::exc

codon/parser/visitors/scoping/scoping.cpp

@@ -38,6 +38,8 @@ llvm::Error ScopingVisitor::apply(Cache *cache, Stmt *s) {
   ConditionalBlock cb(c.get(), s, 0);
   if (!v.transform(s))
     return llvm::make_error<ParserErrorInfo>(v.errors);
+  if (v.hasErrors())
+    return llvm::make_error<ParserErrorInfo>(v.errors);
   v.processChildCaptures();
   return llvm::Error::success();
 }
@@ -45,8 +47,10 @@ llvm::Error ScopingVisitor::apply(Cache *cache, Stmt *s) {
 bool ScopingVisitor::transform(Expr *expr) {
   ScopingVisitor v(*this);
   if (expr) {
-    setSrcInfo(expr->getSrcInfo());
+    v.setSrcInfo(expr->getSrcInfo());
     expr->accept(v);
+    if (v.hasErrors())
+      errors.append(v.errors);
     if (!canContinue())
       return false;
   }
@@ -56,8 +60,10 @@ bool ScopingVisitor::transform(Expr *expr) {
 bool ScopingVisitor::transform(Stmt *stmt) {
   ScopingVisitor v(*this);
   if (stmt) {
-    setSrcInfo(stmt->getSrcInfo());
+    v.setSrcInfo(stmt->getSrcInfo());
     stmt->accept(v);
+    if (v.hasErrors())
+      errors.append(v.errors);
     if (!canContinue())
       return false;
   }
@@ -102,6 +108,8 @@ bool ScopingVisitor::transformAdding(Expr *e, ASTNode *root) {
 }
 
 void ScopingVisitor::visit(IdExpr *expr) {
+  if (ctx->adding)
+    ctx->root = expr;
   if (ctx->adding && ctx->tempScope)
     ctx->renames.back()[expr->getValue()] =
         ctx->cache->getTemporaryVar(expr->getValue());
@@ -301,7 +309,7 @@ void ScopingVisitor::visit(ForStmt *stmt) {
     ConditionalBlock c(ctx.get(), stmt->getSuite());
 
     ctx->scope.back().seenVars = std::make_unique<std::unordered_set<std::string>>();
-    CHECK(transformAdding(stmt->var, stmt));
+    CHECK(transformAdding(stmt->getVar(), stmt));
     seenDef = *(ctx->scope.back().seenVars);
 
     ctx->scope.back().seenVars = std::make_unique<std::unordered_set<std::string>>();
@@ -336,7 +344,6 @@ void ScopingVisitor::visit(ImportStmt *stmt) {
   }
   if (ctx->functionScope && stmt->getWhat() && isId(stmt->getWhat(), "*"))
     STOP_ERROR(error::Error::IMPORT_STAR, stmt);
-  return;
 
   // dylib C imports
   if (stmt->getFrom() && isId(stmt->getFrom(), "C") && cast<DotExpr>(stmt->getWhat()))

codon/parser/visitors/translate/translate.cpp

@@ -532,7 +532,7 @@ void TranslateVisitor::visit(ForStmt *stmt) {
   seqassert(cast<IdExpr>(stmt->getVar()), "expected IdExpr, got {}", *(stmt->getVar()));
   auto varName = cast<IdExpr>(stmt->getVar())->getValue();
   ir::Var *var = nullptr;
-  if (!ctx->find(varName) || !stmt->hasAttribute(Attr::ExprDominated)) {
+  if (!ctx->find(varName) || !stmt->getVar()->hasAttribute(Attr::ExprDominated)) {
     var =
         make<ir::Var>(stmt, getType(stmt->getVar()->getType()), false, false, varName);
   } else {

codon/parser/visitors/typecheck/access.cpp

@@ -518,7 +518,7 @@ Expr *TypecheckVisitor::getClassMember(DotExpr *expr) {
   }
 
   // For debugging purposes:
-  // findMethod(typ.get(), expr->getMember());
+  findMethod(typ, expr->getMember());
   E(Error::DOT_NO_ATTR, expr, typ->prettyString(), expr->getMember());
   return nullptr;
 }

codon/parser/visitors/typecheck/assign.cpp

@@ -36,8 +36,8 @@ void TypecheckVisitor::visit(AssignStmt *stmt) {
   }
 
   bool mustUpdate = stmt->isUpdate() || stmt->isAtomicUpdate();
-  mustUpdate |= stmt->hasAttribute(Attr::ExprDominated);
-  mustUpdate |= stmt->hasAttribute(Attr::ExprDominatedUsed);
+  mustUpdate |= stmt->getLhs()->hasAttribute(Attr::ExprDominated);
+  mustUpdate |= stmt->getLhs()->hasAttribute(Attr::ExprDominatedUsed);
   if (cast<BinaryExpr>(stmt->getRhs()) &&
       cast<BinaryExpr>(stmt->getRhs())->isInPlace()) {
     // Update case: a += b
@@ -46,9 +46,9 @@ void TypecheckVisitor::visit(AssignStmt *stmt) {
   }
 
   resultStmt = transformAssignment(stmt, mustUpdate);
-  if (stmt->hasAttribute(Attr::ExprDominatedUsed)) {
+  if (stmt->getLhs()->hasAttribute(Attr::ExprDominatedUsed)) {
     // If this is dominated, set __used__ if needed
-    stmt->eraseAttribute(Attr::ExprDominatedUsed);
+    stmt->getLhs()->eraseAttribute(Attr::ExprDominatedUsed);
     auto e = cast<IdExpr>(stmt->getLhs());
     seqassert(e, "dominated bad assignment");
     resultStmt = transform(N<SuiteStmt>(
@@ -108,6 +108,8 @@ Stmt *TypecheckVisitor::unpackAssignment(Expr *lhs, Expr *rhs) {
     // Case: [a, b] = ...
     for (auto *i : *el)
       leftSide.push_back(i);
+  } else {
+    return N<AssignStmt>(lhs, rhs);
   }
 
   // Prepare the right-side expression

codon/parser/visitors/typecheck/call.cpp

@@ -566,7 +566,7 @@ Expr *TypecheckVisitor::callReorderArguments(FuncType *calleeFn, CallExpr *expr,
     reorderNamedArgs(
         calleeFn, expr->items, reorderFn,
         [&](error::Error e, const SrcInfo &o, const std::string &errorMsg) {
-          E(e, o, errorMsg);
+          E(Error::CUSTOM, o, errorMsg.c_str());
           return -1;
         },
         part.known);

codon/parser/visitors/typecheck/collections.cpp

@@ -126,7 +126,6 @@ void TypecheckVisitor::visit(GeneratorExpr *expr) {
     block->addStmt(N<AssignStmt>(N<IdExpr>(tupleVar), gen));
 
     auto forStmt = clone(cast<ForStmt>(expr->getFinalSuite()));
-    seqassert(cast<IdExpr>(forStmt->getVar()), "tuple() not simplified");
     auto finalExpr = expr->getFinalExpr();
     auto [ok, delay, preamble, staticItems] = transformStaticLoopCall(
         cast<ForStmt>(expr->getFinalSuite())->getVar(), &forStmt->suite, gen,

codon/parser/visitors/typecheck/infer.cpp

@@ -162,7 +162,8 @@ types::Type *TypecheckVisitor::realize(types::Type *typ) {
       return t;
     }
   } catch (exc::ParserException &exc) {
-    auto &bt = exc.getErrors().getLast();
+    seqassert(!exc.getErrors().empty(), "empty error trace");
+    auto &bt = exc.getErrors().back();
     if (bt.front().getErrorCode() == Error::MAX_REALIZATION)
       throw;
     if (auto f = typ->getFunc()) {

codon/parser/visitors/typecheck/loops.cpp

@@ -116,14 +116,6 @@ void TypecheckVisitor::visit(ForStmt *stmt) {
   if (!iterType)
     return; // wait until the iterator is known
 
-  // Replace for (i, j) in ... { ... } with for tmp in ...: { i, j = tmp ; ... }
-  if (!cast<IdExpr>(stmt->getVar())) {
-    auto var = N<IdExpr>(ctx->cache->getTemporaryVar("for"));
-    auto ns = unpackAssignment(stmt->getVar(), var);
-    stmt->suite = N<SuiteStmt>(ns, stmt->getSuite());
-    stmt->var = var;
-  }
-
   auto [delay, staticLoop] = transformStaticForLoop(stmt);
   if (delay)
     return;
@@ -132,6 +124,14 @@ void TypecheckVisitor::visit(ForStmt *stmt) {
     return;
   }
 
+  // Replace for (i, j) in ... { ... } with for tmp in ...: { i, j = tmp ; ... }
+  if (!cast<IdExpr>(stmt->getVar())) {
+    auto var = N<IdExpr>(ctx->cache->getTemporaryVar("for"));
+    auto ns = unpackAssignment(stmt->getVar(), var);
+    stmt->suite = N<SuiteStmt>(ns, stmt->getSuite());
+    stmt->var = var;
+  }
+
   // Case: iterating a non-generator. Wrap with `__iter__`
   if (iterType->name != "Generator" && !stmt->isWrapped()) {
     stmt->iter = transform(N<CallExpr>(N<DotExpr>(stmt->getIter(), "__iter__")));
@@ -143,13 +143,13 @@ void TypecheckVisitor::visit(ForStmt *stmt) {
   auto var = cast<IdExpr>(stmt->getVar());
   seqassert(var, "corrupt for variable: {}", *(stmt->getVar()));
 
-  if (!stmt->hasAttribute(Attr::ExprDominated) &&
-      !stmt->hasAttribute(Attr::ExprDominatedUsed)) {
+  if (!var->hasAttribute(Attr::ExprDominated) &&
+      !var->hasAttribute(Attr::ExprDominatedUsed)) {
     ctx->addVar(var->getValue(), ctx->generateCanonicalName(var->getValue()),
                 instantiateUnbound());
-  } else if (stmt->hasAttribute(Attr::ExprDominatedUsed)) {
-    stmt->eraseAttribute(Attr::ExprDominatedUsed);
-    stmt->setAttribute(Attr::ExprDominated);
+  } else if (var->hasAttribute(Attr::ExprDominatedUsed)) {
+    var->eraseAttribute(Attr::ExprDominatedUsed);
+    var->setAttribute(Attr::ExprDominated);
     stmt->suite = N<SuiteStmt>(
         N<AssignStmt>(N<IdExpr>(format("{}{}", var->getValue(), VAR_USED_SUFFIX)),
                       N<BoolExpr>(true), nullptr, AssignStmt::UpdateMode::Update),
@@ -288,37 +288,26 @@ TypecheckVisitor::transformStaticLoopCall(Expr *varExpr, SuiteStmt **varSuite,
   if (!iter->getClassType())
     return {true, true, nullptr, {}};
 
-  seqassert(cast<IdExpr>(varExpr), "bad varExpr");
-  std::function<int(Stmt **, const std::function<void(Stmt **)> &)> iterFn;
-  iterFn = [&iterFn](Stmt **s, const std::function<void(Stmt **)> &fn) -> int {
-    if (!s)
-      return 0;
-    if (auto su = cast<SuiteStmt>(*s)) {
-      int i = 0;
-      for (auto &si : *su) {
-        i += iterFn(&si, fn);
-      }
-      return i;
+  std::vector<std::string> vars{};
+  if (auto ei = cast<IdExpr>(varExpr)) {
+    vars.push_back(ei->getValue());
+  } else {
+    Items<Expr *> *list = nullptr;
+    if (auto el = cast<ListExpr>(varExpr))
+      list = el;
+    else if (auto et = cast<TupleExpr>(varExpr))
+      list = et;
+    if (list) {
+      for (const auto &it : *list)
+        if (auto ei = cast<IdExpr>(it)) {
+          vars.push_back(ei->getValue());
+        } else {
+          return {false, false, nullptr, {}};
+        }
     } else {
-      fn(s);
-      return 1;
+      return {false, false, nullptr, {}};
     }
-  };
-  std::vector<std::string> vars{cast<IdExpr>(varExpr)->getValue()};
-  iterFn((Stmt **)varSuite, [&](Stmt **s) {
-    // Handle iteration var transformations (for i, j in x -> for _ in x: (i, j = _;
-    // ...))
-    IdExpr *var = nullptr;
-    if (match(*s, M<AssignStmt>(MVar<IdExpr>(var), M<IndexExpr>(M<IdExpr>(vars[0]), M_),
-                                M_, M_))) {
-      vars.push_back(var->getValue());
-      *s = nullptr;
-    }
-  });
-  if (vars.size() > 1)
-    vars.erase(vars.begin());
-  if (vars.empty())
-    return {false, false, nullptr, {}};
+  }
 
   Stmt *preamble = nullptr;
   auto fn =

codon/parser/visitors/typecheck/special.cpp

@@ -359,8 +359,11 @@ Expr *TypecheckVisitor::transformNamedTuple(CallExpr *expr) {
   }
   for (auto &g : generics)
     params.push_back(g);
-  prependStmts->push_back(transform(
-      N<ClassStmt>(name, params, nullptr, std::vector<Expr *>{N<IdExpr>("tuple")})));
+  auto cls = N<SuiteStmt>(
+      N<ClassStmt>(name, params, nullptr, std::vector<Expr *>{N<IdExpr>("tuple")}));
+  if (auto err = ast::ScopingVisitor::apply(ctx->cache, cls))
+    throw exc::ParserException(std::move(err));
+  prependStmts->push_back(transform(cls));
   return transformType(N<IdExpr>(name));
 }
 
@@ -680,7 +683,7 @@ Expr *TypecheckVisitor::transformSetAttr(CallExpr *expr) {
 /// Raise a compiler error.
 Expr *TypecheckVisitor::transformCompileError(CallExpr *expr) {
   auto msg = getStrLiteral(extractFuncGeneric(expr->expr->getType()));
-  E(Error::CUSTOM, expr, msg);
+  E(Error::CUSTOM, expr, msg.c_str());
   return nullptr;
 }
 

test/main.cpp

@@ -288,7 +288,6 @@ public:
                               fflush(stdout);
                               exit(EXIT_FAILURE);
                             });
-
       auto *pm = compiler->getPassManager();
       pm->registerPass(std::make_unique<TestOutliner>());
       pm->registerPass(std::make_unique<TestInliner>());
@@ -300,7 +299,6 @@ public:
                                 ir::analyze::dataflow::DominatorAnalysis::KEY});
       pm->registerPass(std::make_unique<EscapeValidator>(capKey), /*insertBefore=*/"",
                        {capKey});
-
       llvm::cantFail(compiler->compile());
       seq_exc_init(0);
       compiler->getLLVMVisitor()->run({file});
@@ -348,6 +346,8 @@ TEST_P(SeqTest, Run) {
     status = runInChildProcess();
   else
     status = runInChildProcess();
+  if (!WIFEXITED(status))
+    std::cerr << result() << std::endl;
   ASSERT_TRUE(WIFEXITED(status));
 
   string output = result();

test/parser/typecheck/test_basic.codon

@@ -73,10 +73,10 @@ print(c, f'{b}{a}', f'. {1+a=} .. {b} ...')
 #: and this is 1 followed by 2 21 . 1+a=2 .. 2 ...
 
 #%% fstring_error_1,barebones
-f"a{b + 3}}" #! single '}' is not allowed in f-string
+f"a{1 + 3}}" #! single '}' is not allowed in f-string
 
 #%% fstring_error_2,barebones
-f"a{{b + 3}" #! expecting '}' in f-string
+f"a{{1 + 3}" #! expecting '}' in f-string
 
 #%% string_prefix,barebones
 @extend