Crystal: Class variable reassignment mistakenly detects initialization recursion

[Exilor]

class Foo
  @@bar = 1
  @@bar += 1
end

Crashes with Unhandled exception: Recursion while initializing class variables and/or constants (Exception).

In 0.30.1 the execution times out. Before that version it worked and after it, stopped working.

[asterite]

I think the error is fine. That is, the current behavior is correct.

The last initialization of a class variable wins. So the code is basically:

class Foo
  @@bar = @@bar + 1
end

Which shouldn't work because @@bar has no value when you want to read it so it tries to be initialized and so on.

However, a problem is that @@bar = 1 is taken into account for the type. It shouldn't be considered.

That said... I wouldn't recommend writing such code, it's a bit confusing.

[asterite]

We should probably also disallow using += and such for class/instance vars at that point.

In fact, I would disallow all top-level code inside classes/modules. It's super confusing and usually leads to these "bugs" in the compiler.

[Exilor]

It's disheartening to see how often removing features of the language is the go-to solution.

[straight-shoota]

@Exilor Why do you think so? IMO it can really be considered an enhancement when it becomes clear that a feature can be removed: Less features -> simpler language -> easier to learn and use. (also it's better to remove a feature when we know it's not working correctly; can always be added later)
Obviously, this is counterweight by usability concerns. But in the case of the example in this issue, I really don't think there'd be a big loss. It's not like this is used very much at all, and it's super easy to move the code to either unscoped top-level or to a class method.

However, I'd like to point out that completely removing top-level code inside a class/module is probably not feasibly. Macros like property or def_equals_and_hash still need to be callable. And methods are similar. Macro and method calls can't really be separated anyway.

[Exilor]

@straight-shoota I'm of the opinion that the similarity with Ruby is Crystal's main selling point and that which keeps it from getting lost in a sea of niche, hobby languages not worth investing time into learning.
There is indeed such thing as feature bloat but I don't think Crystal is remotely approaching that point especially after everything that had to be dropped due to its static nature.
Also on a more selfish level I find it inconvenient to have to work around having a functionality taken away from me for what I believe to be no good reason.

[Sija]

The way I see it you're both right in your own way :)

@straight-shoota has a point about leaving only necessary features and trying to reduce the language to its _core_, while leaving the rest in the hands of capable 3rd party developers, as I understand, in the vein of _less is more_ principle.

@Exilor OTOH I feel mainly relates to the attitude of removing every language feature that seems broken (@asterite, looking at you ;)) - which as time and experience has shown, is sometimes a bit hasty (@asterite, do u remember these "impossible" things you've done in the past?)

Personally, I'd love to see some formal language spec, which could be a reference point for future planning. RFCs are IMO a step in good direction, but many of the fundamental changes still happens in kind of an YOLO style... which I like btw, although in terms of language design that might not work that well. dois centavos de mim

[asterite]

My reasoning is this:

class Foo
  @x = 1
  puts "Hello!"
end

In that code, @x = 1 is executed every time Foo is instantiated. But puts "Hello!" happens just once when the program starts.

I think that's confusing.

We can't remove @x = 1 because... well, we could, but we also need @x : Int32 to specify the type of instance variable, like you do in statically typed languages. And we added @x = 1 and @@x = 1 because it's useful. However, I see that we have two things that behave in completely different ways.

Now, I'm not saying that we should remove initialization code at the class level. I'm just saying that throwing code like that at the class level should be an error. Instead, we should have proper initialization like in other languages. Java has static() { ... }. C# has something similar. I think Go too (for packages). It's clear that those things happens at one specific point. But right now having @x = 1 being executed in one point but puts "Hello" being executed in another is confusing. Furthermore, if you have this:

begin
  puts "Hello!"
  @@x = 1
end

is completely different from just doing @@x = 1.

Now, if you don't think that's confusing I guess it's fine to leave the language like this.

[Sija]

Now, if you don't think that's confusing I guess it's fine to leave the language like this.

I'd agree, but then being able to call macros or static methods from within the class body is sth desirable. Same goes for Ruby btw. Re-opening classes can be considered by some as an anti-pattern but here it's up to the developer to make his own decisions in regards to the way he chooses to use these features...

[RX14]

Making people put it inside a begin...end when inside class seems reasonable, since then we can disambiguate between serial code which does not take part in type inference, and variable definitions.

I think we could also completely disallow overriding instance var and cvar definitions inside the same class definition (they would be allowed on reopen)