+= doesn't seem to work on a property of a struct

[Fryguy]

Here's a sample of the code showing the problem.

class Example
  struct Thing
    property count = 0
  end

  property thing = Thing.new

  def foo
    puts thing.count # => 0
    thing.count += 1
    puts thing.count # => 0
  end
end

Example.new.foo

If I change the struct to a class, everything works fine. I understand structs are allocated differently, and act differently. The docs say " The rule of thumb is that if no instance variable is ever reassigned, i.e. your type is immutable, you could use a struct, otherwise use a class.", though I don't truly understand that rule of thumb and there is clearly mutation here. Even so, this was completely unexpected.

As expected, it's not just +=, but also -=, *=, /= and probably others.

---

Crystal 0.23.1 (2017-07-13) LLVM 4.0.1 macOS Sierra 10.12.6

[c910335]

According to normalizer.cr#L215, thing.count += 1 will be transformed to

tmp = thing
tmp.count = (tmp.count + 1)

However, since a struct is passed by value (Structs), thing.count += 1 only increases the duplicate of thing.count.

In conclusion, this is a bug.

[RX14]

this is a duplicate of some issue years ago which will explain why this happens and why it hasn't been fixed.

[konovod]

I know of #3073 (maybe not literally duplicate, but similar discussion).

Personally i just use pointers to structs for "mutable structs", using property in struct is just asking for problems. On the other hand, i'm happy that mutable structs are possible at all, as they provide a way to work without heap allocations.

[asterite]

It's strange because if you do:

thing.count = thing.count + 1

it does work. So definitely a bug. I'll try to fix it.

[asterite]

Oooh... but it's because this:

foo.bar += 1

actually gets rewritten to this:

tmp = foo
tmp.bar = tmp.bar + 1

That's because you don't want to evaluate foo twice, specially if it has side effects.

So I don't think this will be fixed. Solutions:

1. Don't use mutable structs, use a class
2. Use foo.bar = foo.bar + 1
3. Use foo.bar.inc!

[bew]

@asterite or make the distinction between a call and a variable?

So if foo is a call, use the temporary variable, and if it's not a call, use it directly?

[asterite]

Well, foo is a call in the original example. It's thing, which is calling the method thing...

[asterite]

Now, this could be fixed if the above expansion didn't happen if the compiler knew that thing is actually a method without side effects. For the case of methods whose body is just an instance variable read this is the case. So this situation can definitely be improved. We can leave this open, there might be an easy fix after all.

[RX14]

I'm sure we went through this exact comment sequence about a year or two ago in https://github.com/crystal-lang/crystal/issues/3073 :)

I don't think that adjusting semantics of the caller based on the implementation of the callee is a good idea for encapsulation reasons. Perhaps this would be a worthy exception o the rule but I still don't like it.

[Fryguy]

What if

foo.bar += 1

were to be rewritten to

tmp = foo.bar
foo.bar = tmp + 1

If so, then with a "normal" +=

x += 1

would be rewritten to

tmp = x
x = tmp + 1

which I assume LLVM would optimize away (or perhaps the compiler could detect that and optimize it away)? I know I'm not covering all of the possible cases here, just trying to propose an alternate idea.

[bew]

@Fryguy I think it won't help, as the goal is to prevent foo from being called twice:

tmp = foo.bar     # call foo here
foo.bar = tmp + 1 # call foo here too

[Fryguy]

Oh fair point...For some reason I was focusing on not calling bar twice :smile:

[ghost]

Use foo.bar = foo.bar + 1

Not sure that this would be a good idea. It would seem to require extra cognitional load onto the programmer if the programmer only wants to add += 1.

[straight-shoota]

@shevegen Then he should use a mutable object.

[Fryguy]

I think the problem with stating "then use a mutable object" is that structs are actually mutable. If, as a language, structs are expected to be immutable, then they should be made immutable.

My only concern here is that += works in an unexpected way, and like Ruby, I think Crystal should subscribe to the Principle of Least Surprise.

[Fryguy]

That's because you don't want to evaluate foo twice, specially if it has side effects.

@asterite When I change the struct to a class, everything works, but in both cases thing is still a property, and thus a method. If we didn't want to evaluate the thing method twice, why would the class/struct switch even matter?

---

Thinking about this more, I'm wondering why should the compiler try to protect the developer from calling thing twice? If the documentation of += (and its siblings) is that it is just syntactic sugar over expanding to whatever = whatever + 1, then the developer would know that foo is being called twice and it would be up to them to care about that or not. Having += deal with special cases in the compiler only makes describing what it does, and thus understanding it by the developer, more complicated.

[straight-shoota]

@Fryguy += is actually intended as a shortcut to not have to write code with a temp variable if you want to avoid accessing a property twice. If you don't care about that, you can easily write foo.bar = foo.bar + 1.

[vegai]

So should += on a struct be a compile error?

[mig-hub]

Just so I understand, what exactly is the reason for not being able to expand to something like this:

tmp = pointerof(foo)
tmp.value.bar = tmp.value.bar + 1

Is it because it would be difficult/slow/expensive to identify cases when it should expand to this instead of the current expansion?

Is it that it is transparently mutating and that is an issue, even though += is clearly a mutation?

Or another reason?

[RX14]

@mig-hub mainly because it hasn't been implemented and it's confusing/magic to the user that this happens.

[mig-hub]

@RX14 Okay I see. For what it's worth I find the magic is introduced by the fact that there is a tmp variable at all, which takes care for us of the fact that we might evaluate foo twice.

I understand both points of view about +=. I too see it in my head as var += x means simply var = var + x and I expect var to be evaluated twice unless I cache it. And I also understand that it is not as simple as that, and if we can do something to silently make sure things are not evaluated twice it is better.

My preference would be to leave this caching-in-a-temporary-variable up to the developer (like @Fryguy proposed), or on the contrary raising an exception if += is used on something judged inappropriate (like @vegai mentioned). But keeping things the way they currently are feels strange.

---

By the way, am I correct in thinking that nothing is immutable in Crystal? (FP definition of immutability) They are only passed as values like in C. The only important bit here is that if you pass the variable to a function/method, it cannot be changed inside this function/method unless you pass a pointer. Right? Because @warent seems to say Structs are immutable.

[RX14]

No, nothing is immutable by force. We simply heavily suggest that struct is only used for immutable datastructures, because they're confusing and slightly fragile to use for these reasons. That's not to say you can't use structs internally to your library/app if you know what you're doing.

[bew]

Could we fix this by changing how += is normalized ? This way:

foo.bar += 2

# if `foo` is a local variable, normalize to:
foo.bar = foo.bar + 2

# if `foo` is a call, normalize to:
tmp = foo
tmp.bar = tmp.bar + 2

EDIT: NEVERMIND, I'm tired, the normalizer already works like that.

---

But what happens with foo.bar.baz += 2 as a chained assignment for a struct, maybe #3073 could help decide which normalization to do?

[bew]

@mig-hub's solution is not possible / doesn't make sense:

tmp = pointerof(foo)
tmp.value.bar = tmp.value.bar + 1

Because in this case foo is a call, and it's technically not possible (doesn't make sense) to take the address of a call. And if we say we take the address of a temp var for the result of foo then it's already too late, the temp var will be a copy of the struct we want.

Note: Now I think I have a solution for #3073 & this issue, I'll post it when I'm ready :stuck_out_tongue:

[ghost]

I was running into this earlier today on gitter and @bew linked me to this thread. After reading asterite's post about solutions, it got me thinking.

Here is an example I made, it would be nice if were able to create the operator +=, like we can with

`, <<, <, <=, ==, ===, !=, =~, !~, >>, >, >=, +, -, *, /, //, !, ~, %, &, |, ^, **, [], []?, []=, <=>, &+, &-, &*, &**

When I was creating my Vector2 struct and encountered this issue, my mind subconsciously / naturally went back to the Gitbook page to check if I was missing the += operator method. I am obviously not a guru, but just wanted to throw this out there