irc: PrimOp memory leak and IR_VERSION mismatch

Signed-off-by: NotAShelf <raf@notashelf.dev>
Change-Id: Iad057cd5f51ef26e7de93ccca7b3d3156a6a6964

src/irc/evaluator.cpp

@@ -69,8 +69,6 @@ struct Evaluator::Impl {
 
   explicit Impl(EvalState& s) : state(s) {}
 
-  // Destructor not needed - unique_ptr handles cleanup automatically
-
   IREnvironment* make_env(IREnvironment* parent = nullptr) {
     auto env = new IREnvironment(parent);
     environments.push_back(std::unique_ptr<IREnvironment>(env));
@@ -99,6 +97,39 @@ struct Evaluator::Impl {
     thunks.erase(v);
   }
 
+  // Copy a forced value into a destination Value
+  void copy_value(Value& dest, Value* src) {
+    if (!src)
+      return;
+    force(src);
+    state.forceValue(*src, noPos);
+    switch (src->type()) {
+    case nInt:
+      dest.mkInt(src->integer());
+      break;
+    case nBool:
+      dest.mkBool(src->boolean());
+      break;
+    case nString:
+      dest.mkString(src->c_str());
+      break;
+    case nPath:
+      dest.mkPath(src->path());
+      break;
+    case nNull:
+      dest.mkNull();
+      break;
+    case nFloat:
+      dest.mkFloat(src->fpoint());
+      break;
+    default:
+      // For attrs, lists, functions, etc., direct assignment is safe
+      // as they use reference counting internally
+      dest = *src;
+      break;
+    }
+  }
+
   void eval_node(const std::shared_ptr<Node>& node, Value& v, IREnvironment* env) {
     if (!node) {
       v.mkNull();
@@ -151,36 +182,7 @@ struct Evaluator::Impl {
       if (!bound) {
         state.error<EvalError>("variable not found").debugThrow();
       }
-      force(bound);
+      copy_value(v, bound);
-      // Copy the forced value's data into v
-      // For simple types, use mk* methods to ensure proper initialization
-      // For complex types (attrs, lists, functions), direct assignment is safe
-      state.forceValue(*bound, noPos);
-      switch (bound->type()) {
-      case nInt:
-        v.mkInt(bound->integer());
-        break;
-      case nBool:
-        v.mkBool(bound->boolean());
-        break;
-      case nString:
-        v.mkString(bound->c_str());
-        break;
-      case nPath:
-        v.mkPath(bound->path());
-        break;
-      case nNull:
-        v.mkNull();
-        break;
-      case nFloat:
-        v.mkFloat(bound->fpoint());
-        break;
-      default:
-        // For attrs, lists, functions, etc., direct assignment is safe
-        // as they use reference counting internally
-        v = *bound;
-        break;
-      }
     } else if (auto* n = node->get_if<LambdaNode>()) {
       auto lambda_env = env;
       auto body = n->body;
@@ -545,37 +547,7 @@ struct Evaluator::Impl {
       auto attr = obj->attrs()->get(sym);
 
       if (attr) {
-        Value* val = attr->value;
+        copy_value(v, attr->value);
-        force(val);
-        // Copy the forced value's data into v
-        // For simple types, use mk* methods to ensure proper initialization
-        // For complex types (attrs, lists, functions), direct assignment is safe
-        state.forceValue(*val, noPos);
-        switch (val->type()) {
-        case nInt:
-          v.mkInt(val->integer());
-          break;
-        case nBool:
-          v.mkBool(val->boolean());
-          break;
-        case nString:
-          v.mkString(val->c_str());
-          break;
-        case nPath:
-          v.mkPath(val->path());
-          break;
-        case nNull:
-          v.mkNull();
-          break;
-        case nFloat:
-          v.mkFloat(val->fpoint());
-          break;
-        default:
-          // For attrs, lists, functions, etc., direct assignment is safe
-          // as they use reference counting internally
-          v = *val;
-          break;
-        }
       } else if (n->default_expr) {
         eval_node(*n->default_expr, v, env);
       } else {

tests/regression_test.cpp

@@ -47,12 +47,11 @@ void test_enum_compatibility() {
               << " (expected 0x34 or 0x33 with WITH=0x32)" << std::endl;
   }
 
-  if (IR_VERSION == 2) {
+  if (IR_VERSION == 3) {
-    std::cout << "  PASS: IR_VERSION bumped to 2 for breaking change" << std::endl;
+    std::cout << "  PASS: IR_VERSION is 3" << std::endl;
-  } else if (static_cast<uint8_t>(NodeType::WITH) == 0x32) {
-    std::cout << "  PASS: IR_VERSION unchanged but WITH restored to 0x32" << std::endl;
   } else {
-    std::cerr << "  FAIL: Either bump IR_VERSION or fix enum values" << std::endl;
+    std::cerr << "  FAIL: IR_VERSION should be 3, got " << IR_VERSION << std::endl;
+    failures++;
   }
 }
 
@@ -272,8 +271,358 @@ void test_float_node() {
              "Float value is approximately 3.14159");
 }
 
+// LambdaPatternNode Tests
+void test_lambda_pattern_simple() {
+  std::cout << "> LambdaPatternNode simple ({ a, b }: a + b)..." << std::endl;
+
+  // Body: a + b (using VarNode for a and b)
+  auto var_a = std::make_shared<Node>(VarNode(0, "a"));
+  auto var_b = std::make_shared<Node>(VarNode(0, "b"));
+  auto body = std::make_shared<Node>(BinaryOpNode(BinaryOp::ADD, var_a, var_b));
+
+  // Create lambda pattern with two required fields
+  LambdaPatternNode lambda_pattern(body);
+  lambda_pattern.required_fields.emplace_back("a", std::nullopt);
+  lambda_pattern.required_fields.emplace_back("b", std::nullopt);
+  lambda_pattern.allow_extra = false;
+
+  auto node = std::make_shared<Node>(std::move(lambda_pattern));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify
+  auto* loaded_node = loaded.entry->get_if<LambdaPatternNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is LambdaPatternNode");
+  TEST_CHECK(loaded_node && loaded_node->required_fields.size() == 2, "Has 2 required fields");
+  TEST_CHECK(loaded_node && loaded_node->optional_fields.size() == 0, "Has 0 optional fields");
+  TEST_CHECK(loaded_node && loaded_node->required_fields[0].name == "a", "First field is 'a'");
+  TEST_CHECK(loaded_node && loaded_node->required_fields[1].name == "b", "Second field is 'b'");
+  TEST_CHECK(loaded_node && !loaded_node->at_binding.has_value(), "No at-binding");
+  TEST_CHECK(loaded_node && !loaded_node->allow_extra, "No ellipsis");
+  TEST_CHECK(loaded_node && loaded_node->body != nullptr, "Has body");
+}
+
+void test_lambda_pattern_with_defaults() {
+  std::cout << "> LambdaPatternNode with defaults ({ a, b ? 10 }: a + b)..." << std::endl;
+
+  // Default value for b
+  auto default_b = std::make_shared<Node>(ConstIntNode(10));
+
+  // Body: a + b
+  auto var_a = std::make_shared<Node>(VarNode(0, "a"));
+  auto var_b = std::make_shared<Node>(VarNode(0, "b"));
+  auto body = std::make_shared<Node>(BinaryOpNode(BinaryOp::ADD, var_a, var_b));
+
+  // Create lambda pattern
+  LambdaPatternNode lambda_pattern(body);
+  lambda_pattern.required_fields.emplace_back("a", std::nullopt);
+  lambda_pattern.optional_fields.emplace_back("b", default_b);
+  lambda_pattern.allow_extra = false;
+
+  auto node = std::make_shared<Node>(std::move(lambda_pattern));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify
+  auto* loaded_node = loaded.entry->get_if<LambdaPatternNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is LambdaPatternNode");
+  TEST_CHECK(loaded_node && loaded_node->required_fields.size() == 1, "Has 1 required field");
+  TEST_CHECK(loaded_node && loaded_node->optional_fields.size() == 1, "Has 1 optional field");
+  TEST_CHECK(loaded_node && loaded_node->required_fields[0].name == "a", "Required field is 'a'");
+  TEST_CHECK(loaded_node && loaded_node->optional_fields[0].name == "b", "Optional field is 'b'");
+  TEST_CHECK(loaded_node && loaded_node->optional_fields[0].default_value.has_value(),
+             "Optional field has default");
+
+  if (loaded_node && loaded_node->optional_fields[0].default_value) {
+    auto* def_val = (*loaded_node->optional_fields[0].default_value)->get_if<ConstIntNode>();
+    TEST_CHECK(def_val && def_val->value == 10, "Default value is 10");
+  }
+}
+
+void test_lambda_pattern_at_binding() {
+  std::cout << "> LambdaPatternNode with at-binding (args@{ a, b }: args.a)..." << std::endl;
+
+  // Body: args.a (select expression)
+  auto var_args = std::make_shared<Node>(VarNode(0, "args"));
+  auto attr = std::make_shared<Node>(ConstStringNode("a"));
+  auto body = std::make_shared<Node>(SelectNode(var_args, attr));
+
+  // Create lambda pattern with at-binding
+  LambdaPatternNode lambda_pattern(body);
+  lambda_pattern.required_fields.emplace_back("a", std::nullopt);
+  lambda_pattern.required_fields.emplace_back("b", std::nullopt);
+  lambda_pattern.at_binding = "args";
+  lambda_pattern.allow_extra = false;
+
+  auto node = std::make_shared<Node>(std::move(lambda_pattern));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify
+  auto* loaded_node = loaded.entry->get_if<LambdaPatternNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is LambdaPatternNode");
+  TEST_CHECK(loaded_node && loaded_node->at_binding.has_value(), "Has at-binding");
+  TEST_CHECK(loaded_node && loaded_node->at_binding.value() == "args", "At-binding is 'args'");
+}
+
+void test_lambda_pattern_ellipsis() {
+  std::cout << "> LambdaPatternNode with ellipsis ({ a, ... }: a)..." << std::endl;
+
+  // Body: a
+  auto body = std::make_shared<Node>(VarNode(0, "a"));
+
+  // Create lambda pattern with ellipsis
+  LambdaPatternNode lambda_pattern(body);
+  lambda_pattern.required_fields.emplace_back("a", std::nullopt);
+  lambda_pattern.allow_extra = true;
+
+  auto node = std::make_shared<Node>(std::move(lambda_pattern));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify
+  auto* loaded_node = loaded.entry->get_if<LambdaPatternNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is LambdaPatternNode");
+  TEST_CHECK(loaded_node && loaded_node->allow_extra, "Has ellipsis (allow_extra=true)");
+}
+
+void test_lambda_pattern_complete() {
+  std::cout << "> LambdaPatternNode complete (args@{ a, b ? 5, ... }: body)..." << std::endl;
+
+  // Default value for b
+  auto default_b = std::make_shared<Node>(ConstIntNode(5));
+
+  // Body: simple var
+  auto body = std::make_shared<Node>(VarNode(0, "x"));
+
+  // Create lambda pattern with all features
+  LambdaPatternNode lambda_pattern(body);
+  lambda_pattern.required_fields.emplace_back("a", std::nullopt);
+  lambda_pattern.optional_fields.emplace_back("b", default_b);
+  lambda_pattern.at_binding = "args";
+  lambda_pattern.allow_extra = true;
+
+  auto node = std::make_shared<Node>(std::move(lambda_pattern));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify all fields
+  auto* loaded_node = loaded.entry->get_if<LambdaPatternNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is LambdaPatternNode");
+  TEST_CHECK(loaded_node && loaded_node->required_fields.size() == 1, "Has 1 required field");
+  TEST_CHECK(loaded_node && loaded_node->optional_fields.size() == 1, "Has 1 optional field");
+  TEST_CHECK(loaded_node && loaded_node->at_binding.has_value(), "Has at-binding");
+  TEST_CHECK(loaded_node && loaded_node->at_binding.value() == "args", "At-binding is 'args'");
+  TEST_CHECK(loaded_node && loaded_node->allow_extra, "Has ellipsis");
+}
+
+void test_lambda_pattern_empty() {
+  std::cout << "> LambdaPatternNode empty ({ }: body)..." << std::endl;
+
+  // Body: simple constant
+  auto body = std::make_shared<Node>(ConstIntNode(42));
+
+  // Create empty lambda pattern
+  LambdaPatternNode lambda_pattern(body);
+  lambda_pattern.allow_extra = false;
+
+  auto node = std::make_shared<Node>(std::move(lambda_pattern));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify
+  auto* loaded_node = loaded.entry->get_if<LambdaPatternNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is LambdaPatternNode");
+  TEST_CHECK(loaded_node && loaded_node->required_fields.size() == 0, "Has 0 required fields");
+  TEST_CHECK(loaded_node && loaded_node->optional_fields.size() == 0, "Has 0 optional fields");
+  TEST_CHECK(loaded_node && !loaded_node->at_binding.has_value(), "No at-binding");
+  TEST_CHECK(loaded_node && !loaded_node->allow_extra, "No ellipsis");
+}
+
+// StringInterpolationNode Tests
+
+void test_string_interpolation_simple() {
+  std::cout << "> StringInterpolationNode simple (\"hello ${name}\")..." << std::endl;
+
+  // "hello ${name}" = literal "hello " + expr(name)
+  std::vector<StringPart> parts;
+  parts.push_back(StringPart::make_literal("hello "));
+  parts.push_back(StringPart::make_expr(std::make_shared<Node>(VarNode(0, "name"))));
+
+  auto node = std::make_shared<Node>(StringInterpolationNode(std::move(parts)));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify
+  auto* loaded_node = loaded.entry->get_if<StringInterpolationNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is StringInterpolationNode");
+  TEST_CHECK(loaded_node && loaded_node->parts.size() == 2, "Has 2 parts");
+  TEST_CHECK(loaded_node && loaded_node->parts[0].type == StringPart::Type::LITERAL,
+             "First part is LITERAL");
+  TEST_CHECK(loaded_node && loaded_node->parts[0].literal == "hello ", "First part is 'hello '");
+  TEST_CHECK(loaded_node && loaded_node->parts[1].type == StringPart::Type::EXPR,
+             "Second part is EXPR");
+  TEST_CHECK(loaded_node && loaded_node->parts[1].expr != nullptr, "Second part has expression");
+}
+
+void test_string_interpolation_multiple() {
+  std::cout << "> StringInterpolationNode multiple (\"${a} and ${b}\")..." << std::endl;
+
+  // "${a} and ${b}" = expr(a) + literal " and " + expr(b)
+  std::vector<StringPart> parts;
+  parts.push_back(StringPart::make_expr(std::make_shared<Node>(VarNode(0, "a"))));
+  parts.push_back(StringPart::make_literal(" and "));
+  parts.push_back(StringPart::make_expr(std::make_shared<Node>(VarNode(0, "b"))));
+
+  auto node = std::make_shared<Node>(StringInterpolationNode(std::move(parts)));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify
+  auto* loaded_node = loaded.entry->get_if<StringInterpolationNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is StringInterpolationNode");
+  TEST_CHECK(loaded_node && loaded_node->parts.size() == 3, "Has 3 parts");
+  TEST_CHECK(loaded_node && loaded_node->parts[0].type == StringPart::Type::EXPR, "Part 0 is EXPR");
+  TEST_CHECK(loaded_node && loaded_node->parts[1].type == StringPart::Type::LITERAL,
+             "Part 1 is LITERAL");
+  TEST_CHECK(loaded_node && loaded_node->parts[1].literal == " and ", "Part 1 is ' and '");
+  TEST_CHECK(loaded_node && loaded_node->parts[2].type == StringPart::Type::EXPR, "Part 2 is EXPR");
+}
+
+void test_string_interpolation_complex() {
+  std::cout << "> StringInterpolationNode complex (\"result: ${a + b}\")..." << std::endl;
+
+  // "result: ${a + b}" = literal "result: " + expr(a + b)
+  auto expr_a = std::make_shared<Node>(VarNode(0, "a"));
+  auto expr_b = std::make_shared<Node>(VarNode(0, "b"));
+  auto add_expr = std::make_shared<Node>(BinaryOpNode(BinaryOp::ADD, expr_a, expr_b));
+
+  std::vector<StringPart> parts;
+  parts.push_back(StringPart::make_literal("result: "));
+  parts.push_back(StringPart::make_expr(add_expr));
+
+  auto node = std::make_shared<Node>(StringInterpolationNode(std::move(parts)));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify
+  auto* loaded_node = loaded.entry->get_if<StringInterpolationNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is StringInterpolationNode");
+  TEST_CHECK(loaded_node && loaded_node->parts.size() == 2, "Has 2 parts");
+  TEST_CHECK(loaded_node && loaded_node->parts[1].type == StringPart::Type::EXPR, "Part 1 is EXPR");
+
+  // Verify the expression is a BinaryOpNode
+  if (loaded_node && loaded_node->parts[1].expr) {
+    auto* bin_op = loaded_node->parts[1].expr->get_if<BinaryOpNode>();
+    TEST_CHECK(bin_op != nullptr, "Expression is BinaryOpNode");
+    TEST_CHECK(bin_op && bin_op->op == BinaryOp::ADD, "Operation is ADD");
+  }
+}
+
+void test_string_interpolation_nested() {
+  std::cout << "> StringInterpolationNode nested (\"${prefix}/${path}\")..." << std::endl;
+
+  // "${prefix}/${path}" = expr(prefix) + literal "/" + expr(path)
+  std::vector<StringPart> parts;
+  parts.push_back(StringPart::make_expr(std::make_shared<Node>(VarNode(0, "prefix"))));
+  parts.push_back(StringPart::make_literal("/"));
+  parts.push_back(StringPart::make_expr(std::make_shared<Node>(VarNode(0, "path"))));
+
+  auto node = std::make_shared<Node>(StringInterpolationNode(std::move(parts)));
+
+  // Serialize
+  IRModule module;
+  module.entry = node;
+  Serializer ser;
+  auto bytes = ser.serialize_to_bytes(module);
+
+  // Deserialize
+  Deserializer deser;
+  auto loaded = deser.deserialize(bytes);
+
+  // Verify
+  auto* loaded_node = loaded.entry->get_if<StringInterpolationNode>();
+  TEST_CHECK(loaded_node != nullptr, "Type is StringInterpolationNode");
+  TEST_CHECK(loaded_node && loaded_node->parts.size() == 3, "Has 3 parts");
+  TEST_CHECK(loaded_node && loaded_node->parts[1].type == StringPart::Type::LITERAL,
+             "Middle part is LITERAL");
+  TEST_CHECK(loaded_node && loaded_node->parts[1].literal == "/", "Middle part is '/'");
+}
+
 int main() {
-  std::cout << "=== Regression Tests for Nixir ===" << std::endl << std::endl;
+  std::cout << "=== Regression Tests ===" << std::endl << std::endl;
 
   test_enum_compatibility();
   std::cout << std::endl;
@@ -308,6 +657,36 @@ int main() {
   test_float_node();
   std::cout << std::endl;
 
+  test_lambda_pattern_simple();
+  std::cout << std::endl;
+
+  test_lambda_pattern_with_defaults();
+  std::cout << std::endl;
+
+  test_lambda_pattern_at_binding();
+  std::cout << std::endl;
+
+  test_lambda_pattern_ellipsis();
+  std::cout << std::endl;
+
+  test_lambda_pattern_complete();
+  std::cout << std::endl;
+
+  test_lambda_pattern_empty();
+  std::cout << std::endl;
+
+  test_string_interpolation_simple();
+  std::cout << std::endl;
+
+  test_string_interpolation_multiple();
+  std::cout << std::endl;
+
+  test_string_interpolation_complex();
+  std::cout << std::endl;
+
+  test_string_interpolation_nested();
+  std::cout << std::endl;
+
   std::cout << "=== Tests Complete ===" << std::endl;
   std::cout << "Failures: " << failures << std::endl;
   return failures > 0 ? 1 : 0;