TypeNode.instance_vars should work everywhere

[vladfaust]

We recently had a discussion in gitter and found out that you can have an access to type instance methods on class level, i.e. https://carc.in/#/r/6evi:

class Foo
  def foo
    "bar"
  end
  
  def baz : String
    "qux"
  end
  
  {% pp @type.methods %}
end

The result is:

[def foo
  "bar"
end, def baz : String
  "qux"
end]

We can see that foo is untyped and that is kind of expected, as of this time the compiler did not revealed its return type yet (shouldn't it, though?).

However, it doesn't work with instance variables (https://carc.in/#/r/6evk):

class Foo
  @foo : Int32 = 0
  {% pp @type.instance_vars %}
end

[]

@asterite has a strong position on that:

Compiler doesn't know all instance variable types yet, that's why it returns empty array.

I see an issue here. The behavior is inconsistent. If we can have an access to instance methods before their types are fully known, why can't we do the same with instance variables? Let them be untyped if types are not know yet, i.e. their .type accessor returns NilLiteral. It should be the developer's responsibility to understand that instance vars aren't fully typed at some moments of time. Macros are somewhat advanced level and I guess it's better to have features which behave differently in different conditions than not having these features at all. Moreover, we already do have such complex feature of accessing instance methods which could or could not be typed at certain moments of time.

Thanks for the attention!

[straight-shoota]

Instance methods returned by the macro reflection API are never typed. Def#return_type simply returns the value specified as the method's return type in code. In fact, Def contains only information available directly from the parser. That's why it can easily be made available directly.

But I agree that it would be great if instance vars could be accessed at the class level - even if they're not fully typed.

[vladfaust]

Is there any chance it gets implemented? It would be a big leap forward for many cool shards in the wild.

[asterite]

No

[vladfaust]

Welp, closing then 👍

[vladfaust]

BTW, if a person spends much time on describing an issue, constructing it, selecting right words and shaving it with Okkam's razor. Then they expect a similar level of effort put into a response. It could be treated as a rude response otherwise.

Writing code is one thing, but formulating problems, finding bugs and proposing features is another and takes time as well.

Such an approach, when an issue persists for ages and many experienced Crystal developers asking for a feature, getting simple "no" as a response. It's devastating and wrong.

I have an argument in my issue, and it's pretty solid (the fact that we have access to methods, but not vars), and I expect to receive a solid counter-argument and explanation on why it's not possible.

[asterite]

Yes, I am on my cellphone, I planned to add an explanation about why this is hard to do.

[asterite]

So, I said "no" because right now the first pass in the compiler doesn't even look at instance variables. We would have to have a separate structure to hold this "yet to be put in the right place" instance vars without types, or something like that. And I don't want to introduce risky changes.

Then, you didn't provide any use case to justify adding this.

Finally, I agree that in the general case if you ask a question then then response should have equal effort put into it, but from my side I want to ask the questions of many people and I simply don't have time to put that much effort into it.

Right now I'lll focus more on fixing existing bugs. I believe the language as it is right now is already super useful, but some bugs are annoying people (and myself) and I'd like to fix them all.

You can also notice that nobody else is working on compiler stuff like this (changing fundamental things), I'm the only one who is doing it right now so me saying "no" also means "I won't do it, so nobody else will (soon)".

[vladfaust]

I'm sorry for putting pressure on you, Ary. Me (and others, for sure) are thankful to you for Crystal, as you're being the one who initially implemented the language and still being one of the most major figures in developing it. I wish you had more time and resources to spend on Crystal (if that is what you really want).

And you're right about the importance of fixing existing bugs either. That's a pity that there are only a few developers able to work on fundamentals.

For me, I've selected a more mass-user-oriented path. I'm working on frameworks which utilize language features in a beautiful but still efficient way. And this, sadly, requires instance variables to be known in compilation time. Otherwise the expressiveness goals I've given to myself aren't reachable.

One of the main Crystal applications in foreseeable future is web development. And web development is often about wrapping databases with API. Having both type-safe-as-possible and enjoyable ORM is crucial for the stack.

Existing solutions suffer from current language limitations, with this very issue being one of the major ones. Knowing instance variables in compilation time would allow to declare methods and arguments needed for enjoyable development experience, avoiding explicit language constructs.

Onyx::SQL is already 2 years-old, and it was rewritten 4 times from scratch IIRC (++25k LoC, --18k LoC). I've tried many approaches to implement the best possible API, and I always faced the need to know them variables. Please believe me on that. Current annotation approach is nearly perfect in the terms of codebase and API, and resolving the issue would make it even more ideal.

For instance, any Query builder method involving a model's attribute referenced by name currently relies on enums autocasting. Enums are defined with a dirty hack based on getters and then restricted in methods like #select. It does bring type-safety, but uglifies the initial code-base and leads to unpleasant developer experience (see #7284).

Another example is Query builder #join. It yields a Query(T) instance, where T depends on the referenced model, therefore I need to know the referenced model type by its name in compilation time. Currently we have to write #join(posts: true) do |subquery| with : true part being absolutely redundant. It's a hack to reveal posts type in compilation time. It would be much better to write just #join(:posts) do |subquery|.

One more use-case is defining throw-away model records with strict types (as an ORM model instance actual variables depend on origin SQL query):

class User
  include Onyx::SQL::Model

  # It can't be NULL in DB, but an SQL query could have it missing in SELECT clause
  @[Onyx::SQL::Field(not_null: true)] 
  property id : Int32?

  @[Onyx::SQL::Field(not_null: true)] # It can't be NULL in DB either
  property name : String?

  property age : Int32?
end

User.record UserObject, id, name

# Expands to (simplified)
struct UserObject
  property id : Int32 # Its type is known from User, and it's `not_null: true`
  property name : String # Ditto
end

user = UserObject.from_rs(db.query("SELECT id, name FROM users"))
user.id # Is never nil

Who knows which features are more to be seen once the issue is resolved properly. I truly hope it gets some love over the time. It's not critical in my case, more of QoL improvement for both sides, but isn't that what we love Crystal for — incredible joy of using of?

[HertzDevil]

If ClassDefs have a macro API, and additionally every TypeNode can query all of its definitions, then it is (relatively) straightforward to iterate the bodies of those definitions and extract the instance variable declarations:

# this assumes the same ivar is never declared more than once
{% for decl in User.definitions %}
  {% body = decl.body %}
  {% if body.is_a?(Expressions) %}
    {% for exp in body.expressions %}
      {% if exp.is_a?(TypeDeclaration) && exp.var.is_a?(InstanceVar) %}
        # `exp.var` is not always a `MacroId`, contrary to the docs
        # `exp.type` is the type restriction of the ivar, which might not correspond to a real type
        # (in contrast, `MetaVar#type` always attempts to resolve the ivar type)
      {% elsif exp.is_a?(Assign) && exp.target.is_a?(InstanceVar) %}
        # ivar initialized in metaclass context with deduced type
      {% elsif exp.is_a?(Def) %}
        # iterate through `exp.body` if necessary, as additional ivars may be declared
      {% end %}
    {% end %}
  {% elsif body.is_a?(TypeDeclaration) && body.var.is_a?(InstanceVar) %}
    # alternatively the macro interpreter could simply wrap singleton
    # expression nodes into `Expressions` so that we don't need this branch
  {% end %}
{% end %}

Alternatively we might add a TypeNode method that does this automatically. Both approaches of course have the same statefulness implications as allowing #instance_vars to return untyped ivars in the first place.