Fix poly-recv attr_writer dispatch (silently dropped assignments)

[OriPekelman]

recv.attr = v on a poly receiver was silently a no-op

When the poly-recv method-dispatch builder
(compile_poly_method_call) walks user classes for an arm to
emit, it has two cases:

1. an explicit method by that name on the class, or
2. an attr_reader / attr_accessor reader fallback.

Both are reader-shaped. The writer side wasn't covered: a class
that exposes flag= only via attr_accessor :flag has no
explicit method named flag=, and the reader fallback only
matches the bare reader name ("flag", not "flag="). So when
the call site landed on poly dispatch, no per-class arm was
emitted at all, the SP_TAG_OBJ block was empty, and the
result temp held its zero-init default. The assignment was
silently elided.

The reader-side worked because it falls into the
cls_has_attr_reader arm. The writer side just had no arm to
fall into.

The bite

Surfaced as a hung dispatch loop in tep, but it's the kind of
bug any code with a polymorphic helper hits:

class Box; attr_accessor :flag; end
class Bag; attr_accessor :flag; end

def clear(o)   # spinel widens `o` to sp_RbVal
  o.flag = false
end

Pre-fix C for o.flag = false:

sp_RbVal _t1 = recv;
mrb_int _t2 = 0;
if (_t1.tag == SP_TAG_OBJ) {
}                              /* <-- empty */
_t2;

Real-world repro: tep's App#dispatch sets req.passed = false
at the top of every iteration of the pass-fallthrough loop.
With this bug, the flag stayed true after the first handler
called pass, every subsequent iteration also "passed", the
loop ran out of routes, and the request 404'd instead of
falling through to the next matching route. (The local
test_pass regression in the tep tree caught it.)

Patch

Two coordinated changes:

1. compile_poly_method_call learns a third arm after the
   reader fallback. When mname ends in = (and isn't ==,
   !=, <=, >=) and the class has an attr_writer for
   the bare name, emit a direct ivar assignment:

   if (recv.cls_id == N) tmp = ((sp_C *)recv.v.p)->iv_x = arg0;

   The result temp gets the assigned value (Ruby semantics).
   Boxing parallels the user-method arm: when the ivar widened
   to poly and the call site supplied a concrete value, box
   via box_value_to_poly.

2. poly_dispatch_return_type also learns the writer case --
   the per-class union of ivar types decides the result
   temp's static type (str / int / float / poly). Without this
   the temp falls through to the int default and a
   const char * = ... arm trips a C compile error.

Regression test: test/poly_attr_writer.rb exercises two
classes sharing an attr_accessor through a function-typed
parameter, with both true→false and false→true transitions.

Reader-only and explicit-def x= paths stay on their existing
arms; this only adds the missing fallback.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces a regression test for polymorphic attribute writers to address a bug where assignments were silently dropped. The review feedback suggests expanding the test coverage to include non-boolean types and verifying the return value of the assignment expression to ensure the fix correctly handles Ruby semantics and avoids potential C compilation errors.

[gemini-code-assist]

The regression test currently only exercises boolean assignments. The PR description mentions that the fix also addresses a C compilation error occurring when the return type of a polymorphic assignment is incorrectly inferred (e.g., when dealing with const char *). To fully verify this fix and the "Ruby semantics" of the return value, the test should be expanded to include non-integer types and verify that the assignment expression returns the correct value.

def trip(o, v); o.flag = v; end

a = Box.new
b = Bag.new

puts trip(a, true) ? "1" : "0"
puts trip(b, "ok")
puts a.flag ? "1" : "0"
puts b.flag

puts trip(a, false) ? "1" : "0"
puts trip(b, false) ? "1" : "0"
puts a.flag ? "1" : "0"
puts b.flag ? "1" : "0"

[gemini-code-assist]

Updated expected output to match the expanded regression test coverage.

1
ok
1
ok
0
0
0
0

[OriPekelman]

Addressed both points from @gemini-code-assist's review:

1. Non-boolean types + return-value capture: test/poly_attr_writer.rb now has three subcases — bool slot, string slot, and a divergent IBox{int}+SBox{string} pair sharing one assign(o, v) call site. Each subcase captures the setter expression's return and prints it (ret=1/ret=0, then a string echo, then the int/string slot reads) — confirms obj.x = v evaluates to v.

2. Divergent slot types: subcase 3 caught a real hole in the original arm. When the rhs is poly (sp_RbVal after the call site widens) and the slot is concrete (mrb_int or const char *), the per-arm write needs to unbox the rhs into the slot's C type — ((sp_IBox *)v.p)->iv_slot = lv_v doesn't compile when lv_v is sp_RbVal. Mirrors the unbox path already in fix(codegen): two silent miscompiles on poly receivers #388. Pre-fix C error:

   error: assigning to 'const char *' from incompatible type 'sp_RbVal'
   

   The arm now matches three rhs/slot shapes:

   • slot poly, rhs concrete → box rhs
   • slot concrete, rhs poly → unbox rhs into slot's type
   • both agree → pass through

Pushed as 4ae9e4a on top of the original commit. Spinel make test 336/0/0; tep make test 91/0 (the test_pass regression — the original tep symptom of this bug — now passes).

[matz]

Thanks Ori! Closing as a duplicate of #388, which is now merged on master.

#388 already includes:

• The third arm in compile_poly_method_call for mname ending in = with cls_has_attr_writer (commit 355f6cf, identical shape).
• The full box / unbox / direct rhs-fitting logic that this PR's 4ae9e4a follow-up adds (commit af1d101 added the same three-case match after the gemini-code-assist review feedback).
• The IBox{int} / SBox{string} divergent-slot regression (PR fix(codegen): two silent miscompiles on poly receivers #388's test/poly_attr_writer.rb covers all three subcases including the bool, string, and divergent-int+string shapes).

Verified on master HEAD (5014bc8): spinel_codegen.rb:28920-28954 has the exact if slot_t == "poly" && raw_t != "poly" -> box / elsif slot_t != "poly" && raw_t == "poly" -> unbox / else direct logic this PR proposes. No coverage gap from closing.

Both PRs were the right diagnosis arrived at independently; #388 happened to land first. Thanks again for the dig!