[Heavy] Fix error messages; Improve create-op syntax

Author: ricejasonf

heavy/include/heavy/Value.h

@@ -1641,7 +1641,7 @@ class Module : public ValueBase {
   }
 
   void Insert(String* Id, String* MangledName) {
-    assert(!MangledName->getStringRef().empty());
+    // assert(!MangledName->getStringRef().empty());
     Map[Id] = MangledName;
   }
 

heavy/include/heavy/mlir.sld

@@ -21,13 +21,13 @@
       (map InitRegion Is BlockArgTypesList UserFns))
 
     (define-syntax create-op
-      (syntax-rules (: loc attributes operands result-types region)
+      (syntax-rules (: loc: attributes: operands: result-types: region:)
         ((create-op Name
-          (loc Loc)
-          (operands Operands ...)
-          (attributes (AttrName Attr) ...)
-          (result-types ResultTypes ...)
-          (region RegionName ((BlockArg : BlockArgType) ...)
+          (loc: Loc)
+          (operands: Operands ...)
+          (attributes: (AttrName Attr) ...)
+          (result-types: ResultTypes ...)
+          (region: RegionName ((BlockArg : BlockArgType) ...)
                     RegionBody ...) ...)
          (let ((Op
                   (old-create-op Name

heavy/lib/Context.cpp

@@ -1302,10 +1302,10 @@ void Context::Raise(Value Obj) {
 
 void Context::RaiseError(String* Msg, llvm::ArrayRef<Value> IrrArgs) {
   Value IrrList = Empty();
-  for (Value Irr : llvm::reverse(IrrArgs))
+  for (Value Irr : llvm::reverse(IrrArgs)) {
     setLoc(Irr.getSourceLocation());
-  for (Value Irr : IrrArgs)
     IrrList = CreatePair(Irr, IrrList);
+  }
   Value Error = CreateError(this->Loc, Msg, IrrList);
   Raise(Error);
 }

heavy/lib/Mlir.cpp

@@ -99,7 +99,7 @@ void create_op_impl(Context& C, ValueRefs Args) {
 
   llvm::StringRef OpName = Args[0].getStringRef();
   if (OpName.empty())
-    return C.RaiseError("expecting operation name");
+    return C.RaiseError("expecting operation name: {}", Args[0]);
 
   Args = Args.drop_front();
 
@@ -114,14 +114,15 @@ void create_op_impl(Context& C, ValueRefs Args) {
     llvm::StringRef Name;
     if (auto* P = dyn_cast<heavy::Pair>(V)) {
       Name = P->Car.getStringRef();
+      if (Name.empty())
+        return C.RaiseError(
+            "expecting name-value pair for attribute: {}", Value(P));
       if (auto* P2 = dyn_cast<heavy::Pair>(P->Cdr)) {
         V = P2->Car;
         if (!isa<heavy::Empty>(P2->Cdr))
           Name = "";  // Clear the name so we raise error below.
       }
     }
-    if (Name.empty())
-      return C.RaiseError("expecting name-value pair for attribute", V);
 
     auto Attr = any_cast<mlir::Attribute>(V);
 
@@ -141,14 +142,14 @@ void create_op_impl(Context& C, ValueRefs Args) {
       for (heavy::Value V2 : V) {
         auto MVal = any_cast<mlir::Value>(V2);
         if (!MVal)
-          return C.RaiseError("expecting mlir.value", V2);
+          return C.RaiseError("expecting mlir.value: {}", V2);
         OpState.operands.push_back(MVal);
       }
       break;
     }
     auto MVal = any_cast<mlir::Value>(V);
     if (!MVal)
-      return C.RaiseError("expecting mlir.value", V);
+      return C.RaiseError("expecting mlir.value: {}", V);
     OpState.operands.push_back(MVal);
   }
 
@@ -160,15 +161,15 @@ void create_op_impl(Context& C, ValueRefs Args) {
   for (heavy::Value V : ResultTypes->getElements()) {
     auto MType = any_cast<mlir::Type>(V);
     if (!MType)
-      return C.RaiseError("expecting mlir.type", V);
+      return C.RaiseError("expecting mlir.type: {}", V);
     OpState.types.push_back(MType);
   }
 
   // successors
   for (heavy::Value V : Successors->getElements()) {
     mlir::Block* Block = any_cast<mlir::Block*>(V);
     if (Block == nullptr)
-      return C.RaiseError("expecting mlir.block", V);
+      return C.RaiseError("expecting mlir.block: {}", V);
     OpState.successors.push_back(Block);
   }
 

heavy/lib/OpGen.cpp

@@ -846,6 +846,8 @@ mlir::Value OpGen::createSyntaxRules(SourceLocation Loc,
 
 // Create a sequence of operations in the current block.
 mlir::Value OpGen::createSequence(SourceLocation Loc, Value Body) {
+  if (CheckError())
+    return mlir::Value();
   if (!isa<Pair>(Body)) {
     return SetError(Loc, "sequence must contain an expression", Body);
   }
@@ -922,6 +924,8 @@ bool OpGen::WalkDefineInits(Value Env, IdSet& LocalIds) {
 }
 
 void OpGen::FinishLocalDefines() {
+  if (CheckError())
+    return;
   TailPosScope TPS(*this);
   IsTailPos = false;
   IsLocalDefineAllowed = false;

heavy/test/Evaluate/create-op.scm

@@ -7,42 +7,42 @@
 ; CHECK: #op{%0 = "heavy.literal"() {info = #heavy<"\22foo\22">} : () -> !heavy.value
 (write
   (create-op "heavy.literal"
-    (loc 0)
-    (operands)
-    (attributes
-      (info (value-attr "foo")))
-    (result-types !heavy.value)))
+    (loc: 0)
+    (operands:)
+    (attributes:
+      ("info" (value-attr "foo")))
+    (result-types: !heavy.value)))
 
 (newline)
 
 ; CHECK: #op{%0 = "heavy.literal"() {info = #heavy<"5">} : () -> !heavy.value
 (write
   (create-op "heavy.literal"
-    (loc 0)
-    (operands)
-    (attributes
-      (info (value-attr 5)))
-    (result-types !heavy.value)
+    (loc: 0)
+    (operands:)
+    (attributes:
+      ("info" (value-attr 5)))
+    (result-types: !heavy.value)
     ))
 
 (newline)
 
 ; CHECK: #op{%0 = "heavy.literal"() {info = #heavy<"5000">} : () -> !heavy.value
 (write
   (create-op "heavy.literal"
-    (loc 0)
-    (operands)
-    (attributes
-      (info (attr "#heavy<\"5000\">" !heavy.value)))
-    (result-types !heavy.value)))
+    (loc: 0)
+    (operands:)
+    (attributes:
+      ("info" (attr "#heavy<\"5000\">" !heavy.value)))
+    (result-types: !heavy.value)))
 
 (define the-answer
   (create-op "heavy.literal"
-    (loc 0)
-    (operands)
-    (attributes
-      (info (value-attr 41)))
-    (result-types !heavy.value)))
+    (loc: 0)
+    (operands:)
+    (attributes:
+      ("info" (value-attr 41)))
+    (result-types: !heavy.value)))
 
 (newline)
 ; CHECK: non-parent:()
@@ -60,31 +60,31 @@
 
 (define command
   (create-op "heavy.command"
-    (loc 0)
-    (operands)
-    (attributes)
-    (result-types)
-    (region "body" ()
+    (loc: 0)
+    (operands:)
+    (attributes:)
+    (result-types:)
+    (region: "body" ()
       (define callee
         (create-op "heavy.load_global"
-          (loc 0)
-          (operands)
-          (attributes
-            (name (flat-symbolref-attr "_HEAVYL5SheavyL4SbaseV5Swrite")))
-          (result-types !heavy.value)
+          (loc: 0)
+          (operands:)
+          (attributes:
+            ("name" (flat-symbolref-attr "_HEAVYL5SheavyL4SbaseV5Swrite")))
+          (result-types: !heavy.value)
           ))
       (define arg1
         (create-op "heavy.literal"
-          (loc 0)
-          (operands)
-          (attributes
-            (info (value-attr 42)))
-          (result-types !heavy.value)))
+          (loc: 0)
+          (operands:)
+          (attributes:
+            ("info" (value-attr 42)))
+          (result-types: !heavy.value)))
       (create-op "heavy.apply"
-        (loc 0)
-        (operands (result callee) (result arg1))
-        (attributes)
-        (result-types))
+        (loc: 0)
+        (operands: (result callee) (result arg1))
+        (attributes:)
+        (result-types:))
       )))
 (write command)
 (newline)