Add set evaluation to eval.ex (Phase 4 of #164)

[andreasronge]

Summary

Add set literal evaluation to the PTC-Lisp evaluator so that #{1 2 3} produces an Elixir MapSet. This enables set literals parsed in Phase 1 and analyzed in Phase 3 to be executed at runtime.

Context

Architecture reference: Set Literal Implementation Plan - Phase 4
Dependencies: Phase 3 (Analyzer) - #170 (merged)
Related issues: Epic #164, Parser #165 (merged), AST #167 (merged), Analyzer #170 (merged)

Current State

The evaluator (lib/ptc_runner/lisp/eval.ex) currently handles:

• Vectors at lines 57-71 using Enum.reduce_while pattern
• Maps at lines 73-84 with key-value pair evaluation

Sets are parsed as {:set, [elements]} AST nodes (verified in analyze.ex:58-62) but there's no evaluation clause in eval.ex. The @type value spec (lines 15-24) doesn't include MapSet.t().

CoreAST already supports sets at line 28: | {:set, [t()]}

Acceptance Criteria

• Evaluator handles {:set, elements} AST nodes
• All set elements are evaluated left-to-right before creating MapSet
• Duplicates are silently removed during MapSet construction (per spec)
• @type value includes MapSet.t()
• Existing tests pass (987 tests currently)
• Unit tests verify set evaluation semantics

Implementation

File to modify: lib/ptc_runner/lisp/eval.ex

1. Update @type value (line 22)

Add MapSet.t() to the value type union:

@type value ::
        nil
        | boolean()
        | number()
        | String.t()
        | atom()
        | list()
        | map()
        | MapSet.t()           # <-- NEW
        | function()
        | {:closure, [CoreAST.pattern()], CoreAST.t(), env()}

2. Add set evaluation clause (after line 84)

Follow the vector pattern from lines 57-71 but wrap result in MapSet.new/1:

# Sets: evaluate all elements, then create MapSet
defp do_eval({:set, elems}, ctx, memory, env, tool_exec) do
  result =
    Enum.reduce_while(elems, {:ok, [], memory}, fn elem, {:ok, acc, mem} ->
      case do_eval(elem, ctx, mem, env, tool_exec) do
        {:ok, v, mem2} -> {:cont, {:ok, [v | acc], mem2}}
        {:error, _} = err -> {:halt, err}
      end
    end)

  case result do
    {:ok, values, memory2} -> {:ok, MapSet.new(values), memory2}
    {:error, _} = err -> err
  end
end

Key difference from vector: Uses MapSet.new(values) instead of Enum.reverse(values).

Test Plan

File: test/ptc_runner/lisp/eval_test.exs

Add a describe "set evaluation" block following the existing describe "map evaluation" pattern:

describe "set evaluation" do
  test "empty set" do
    assert {:ok, result, %{}} = Eval.eval({:set, []}, %{}, %{}, %{}, &dummy_tool/2)
    assert result == MapSet.new([])
  end

  test "set with literals" do
    assert {:ok, result, %{}} = Eval.eval({:set, [1, 2, 3]}, %{}, %{}, %{}, &dummy_tool/2)
    assert MapSet.equal?(result, MapSet.new([1, 2, 3]))
  end

  test "set with mixed types" do
    assert {:ok, result, %{}} =
             Eval.eval(
               {:set, [1, {:string, "test"}, {:keyword, :foo}]},
               %{},
               %{},
               %{},
               &dummy_tool/2
             )

    assert MapSet.equal?(result, MapSet.new([1, "test", :foo]))
  end

  test "nested sets" do
    inner = {:set, [1, 2]}

    assert {:ok, result, %{}} =
             Eval.eval({:set, [inner]}, %{}, %{}, %{}, &dummy_tool/2)

    assert MapSet.equal?(result, MapSet.new([MapSet.new([1, 2])]))
  end

  test "set containing vector" do
    assert {:ok, result, %{}} =
             Eval.eval({:set, [{:vector, [1, 2]}]}, %{}, %{}, %{}, &dummy_tool/2)

    assert MapSet.equal?(result, MapSet.new([[1, 2]]))
  end

  test "set containing nil" do
    assert {:ok, result, %{}} = Eval.eval({:set, [nil, 1, 2]}, %{}, %{}, %{}, &dummy_tool/2)
    assert MapSet.equal?(result, MapSet.new([nil, 1, 2]))
  end
end

describe "set error propagation" do
  test "error in set element propagates" do
    set_ast = {:set, [1, 2, {:var, :unbound}]}

    assert {:error, {:unbound_var, :unbound}} =
             Eval.eval(set_ast, %{}, %{}, %{}, &dummy_tool/2)
  end

  test "error in nested set propagates" do
    nested = {:set, [{:set, [1, {:var, :x}]}]}

    assert {:error, {:unbound_var, :x}} =
             Eval.eval(nested, %{}, %{}, %{}, &dummy_tool/2)
  end
end

describe "set memory threading" do
  test "memory is threaded through set evaluation" do
    memory = %{count: 5}

    {:ok, result, new_memory} =
      Eval.eval({:set, [1, 2, 3]}, %{}, memory, %{}, &dummy_tool/2)

    assert MapSet.equal?(result, MapSet.new([1, 2, 3]))
    assert new_memory == memory
  end
end

Note: Full E2E tests and runtime function tests (set?, contains? on sets, etc.) will be added in Phase 5 (Runtime).

Edge Cases

Case	Expected Result
#{} (empty set)	MapSet.new([])
#{1 2 3}	MapSet.new([1, 2, 3])
#{nil}	MapSet.new([nil]) - valid set containing nil
#{#{1}}	Nested MapSet - MapSet.new([MapSet.new([1])])
#{[1 2]}	MapSet containing a list - MapSet.new([[1, 2]])
Set with unbound var	Propagates {:error, {:unbound_var, name}}

Deduplication behavior: Duplicate values silently coalesce during MapSet.new/1 construction - this happens automatically at the Elixir level.

Out of Scope

• Runtime functions (set?, set, contains? on sets) - Phase 5
• MapSet guards for existing functions (map, filter, contains?, etc.) - Phase 5
• Formatter support - Phase 6
• Documentation updates - Phase 7

Documentation Updates

None required for this phase - internal implementation only. Public documentation will be updated in Phase 7.

[andreasronge]

Issue Review: Add set evaluation to eval.ex (Phase 4 of #164)

Summary: Well-scoped phase implementing set literal evaluation in the PTC-Lisp evaluator. Follows established patterns from vector/map evaluation.

Pre-Review Checks

• Codebase health: ✅ Pass - All 987 tests passing
• Higher-priority blockers: ✅ None - This is part of active epic Set Literal Syntax (#{...}) for PTC-Lisp #164
• Issue claims verified: ✅ Yes - All line numbers and code references accurate

Analysis

1. Should This Be Done? ✅ Yes - Required for set literal feature (epic Set Literal Syntax (#{...}) for PTC-Lisp #164). Phase 3 (feat(analyzer): Add set analysis support (Phase 3 of #164) #170) is merged, this is the natural next step.

2. Simplifications: ✅ Already minimal - Single clause addition following existing pattern.

3. Codebase Research: ✅ Verified:

   • Vector eval at eval.ex:57-71 uses Enum.reduce_while pattern
   • Map eval at eval.ex:73-84 shows similar collection handling
   • CoreAST already defines {:set, [t()]} at core_ast.ex:28
   • Analyzer already handles sets at analyze.ex:58-62

4. Reuse Opportunities: ✅ Excellent - Implementation directly reuses the Enum.reduce_while pattern from vector evaluation.

5. Refactoring Needs: ✅ None - Clean addition point after line 84.

6. Edge Cases: ✅ Covered:

   • Empty set, nested sets, sets with nil
   • Error propagation from unbound variables
   • Memory threading through evaluation

7. Issue Scope: ✅ Appropriate - Single-file change, clear boundaries. Runtime functions properly deferred to Phase 5.

8. Test Plan: ✅ Comprehensive - Tests for basic evaluation, mixed types, nesting, error propagation, and memory threading. Follows existing test patterns.

9. Solution Outline: ✅ Clear - Junior developer could implement from this description. Code snippets provided, exact line numbers referenced.

Changes Made to Issue

• Fixed dependency reference: Add set analysis to analyze.ex (Phase 3 of #164) #169 → feat(analyzer): Add set analysis support (Phase 3 of #164) #170 (Phase 3 PR that was merged)
• Added verification that CoreAST already supports sets (line 28)
• Expanded test plan with complete test code following existing patterns
• Added error propagation and memory threading test cases
• Structured edge cases into a clear table
• Clarified that deduplication happens automatically via MapSet.new/1

Technical Issues Found

None - implementation is straightforward.

Recommended Labels

• ready-for-implementation
• claude-approved

Verdict

Ready - Issue is well-specified, follows established patterns, and has clear acceptance criteria. Implementation can proceed.

[andreasronge]

@claude Please implement this issue.

Guidance:

1. Read the spec document: docs/ptc-lisp-set-literal-plan.md Section 5.4 (Phase 4: Evaluator)
2. Follow the existing vector evaluation pattern in lib/ptc_runner/lisp/eval.ex (lines 57-71)
3. Key implementation points:
   • Update @type value to include MapSet.t()
   • Add do_eval({:set, elems}, ...) clause after map evaluation
   • Use MapSet.new(values) to wrap the evaluated elements
4. Add unit tests in test/ptc_runner/lisp/eval_test.exs
5. Run mix format && mix compile --warnings-as-errors && mix test before PR

[andreasronge]

Implementation Complete

Phase 4 (Evaluator) is now complete and ready for review. PR #172 implements set evaluation with:

Changes

• Updated @type value in eval.ex to include MapSet.t()
• Added do_eval({:set, elems}, ...) clause (lines 87-101 in eval.ex)
  • Uses identical Enum.reduce_while pattern as vector evaluation
  • Collects evaluated elements and wraps in MapSet.new()
  • Handles error propagation correctly

Testing

• 8 new unit tests covering all edge cases from the spec
• Empty sets, literals, deduplication, mixed types, nesting, errors, nil values, memory threading
• All 995 tests pass (including new tests)
• Pre-commit checks all passed

Quality Metrics

• No warnings or compilation errors
• Follows existing patterns for consistency
• Minimal change (15 lines of code added)

Ready for next phase (Runtime Functions).