Decorating getters that return structs by reference

[HertzDevil]

Defs whose body consists of a single InstanceVar return that instance variable by reference, even if it has a value type. The majority of those methods are getters:

struct Foo
  property x = 1
end

struct Bar
  # the def body is just `@foo`, and `Foo` is a mutable struct
  getter foo = Foo.new

  # the def body is just `@y`, and `StaticArray(Int32, 1)` is a mutable value aggregate
  getter y = StaticArray(Int32, 1).new(2)
end

bar = Bar.new
bar.foo.x = 3
bar.y[0] = 4
bar # => Bar(@foo=Foo(@x=3), @y=StaticArray[4])

Owing to the ease of redefining things in Crystal, it is natural, for example, to add a decorator to a getter using previous_def:

require "log"

struct Bar
  def foo
    Log.info { "Bar#foo called" }
    previous_def
  end
end

Now the def body is an Expressions instead of an InstanceVar, thus the semantics of the program have changed, and the last line now prints @foo=Foo(@x=1) printed. In the extreme case, y could have a massize size, and the code even suffers from long compilation times due to passing large structs around by value.

There are times when we cannot opt for setters on the instance variables (e.g. bar.y = StaticArray(Int32, 1).new(4) is a very different piece of code). This seems too big a pitfall in the language to ignore. How do we handle this while preserving the reference semantics? Is there perhaps a better way of writing such decorators?

Related: #13265 (this one is for Object#not_nil!), #3073

---

Add a 👍 reaction to issues you find important.

[straight-shoota]

I find this language feature is quite a quirky oddity. If we could start over, I'd strongly argue to leave it out of the language.
It leads to many surprises.
Arguably, people are often positively surprised that this works. Or it quietly works without them being aware of it.
So that's great. It seems to have some merit and seems to generally work reasonably well.

But it has rough edges and stands out as a weird thing in the language that easily breaks expectations about whether changes have semantic effect.
It's not just adding method decorators that can break it. Any number of inconspicuous changes at the call site can cause significant changes to code semantics.

# assign an intermediary value to a local variable
(f = bar.foo).x = 4
bar.foo.x # => 3 ❌

# add a no-op call into the chain
bar.tap{}.foo.x = 5
bar.foo.x # => 3 ❌

# except when the no-op call _itself_ (haha) qualifies for implicit reference return
bar.itself.foo.x = 6
bar.foo.x # => 6 ✅

It's all a big inconsistency.

Decorators would certainly help improve some potential pitfalls. But there are still many other things that can go wrong. 🤷
I'm wondering if there's could be some other way to make this all more safe and sound. And maybe a bit more explicit.

[ysbaddaden]

Yes, it leads to surprising behavior. I thought it only worked for lib structs as a special convenience to be honest (and I must admit it's nice).