Fix interpreter when inlining pure InstanceVar virtual def and only one concrete subclass exists

[cyangle]

fixes #16278

@ysbaddaden @straight-shoota This fix was generated by google gemini AI via Gemini Code Assist plugin in vscode.

I created this PR to see if the fix would cause any other test to fail.

So please review this carefully.

It seems like this fix is reasonable as now both if branches pass down the target_def.owner.

[ysbaddaden]

I don't enough of the compiler internals to know whether the solution is correct, but CI is happy and the bug seems fixed 👍

[HertzDevil]

In compiled code, When a def's body is exactly an InstanceVar node, Crystal::CodeGenVisitor#try_inline_call inlines calls to that def such that the calls act like a ReadInstanceVar instead, for both improving codegen performance and returning struct members by reference:

struct Foo
  property x = 1
  property y = 2
end

class Bar
  @foo = Foo.new

  def foo
    @foo
  end

  def foo2
    foo # not an `InstanceVar` node
  end
end

bar = Bar.new
bar.foo.x = 3  # this `#foo` call is inlined
bar            # => #<Bar:0x1e65ad36fc0 @foo=Foo(@x=3, @y=2)>
bar.foo2.x = 4 # this `#foo2` call is not inlined
bar            # => #<Bar:0x1e65ad36fc0 @foo=Foo(@x=3, @y=2)>

The interpreter does the same in order to preserve the above semantics, but it fails to consider the case where the call (A#bar from the original issue) has an implicit receiver of self, self is virtual, and only one concrete subclass exists. In this case scope is the virtual type A+, which is different from the sole concrete type B, and it is the latter that should be used.

Calls with an explicit receiver are unaffected, as they trigger the compile_read_instance_var path instead, which already takes an owner distinct from scope. Hence B.new.as(A).foo will not reproduce the original issue.

---

Normally, the onus of providing this description of the problem and solution is on the PR submitter though, so that even reviewers with less knowledge of the compiler's code could be confident that the PR is not merely addressing a symptom while masking the underlying cause, nor throwing code at the wall and seeing what sticks. Using a bot does not grant the free pass of saying that a fix "seems" to be reasonable.

[ysbaddaden]

returning struct members by reference:

Oh, now I understand why a.b.c.d = value does mutate the original value despite using structs (it's pass by ref, and there's no copy involved) ❤️

[cyangle]

@HertzDevil I was just wondering if AI could help this project or not, so I gave it a try.

Below is the chat history including the chain of thoughts, I hope it helps clarifying things a bit. And I think it might be helpful to use AI for analyzing bugs.

@straight-shoota What's the policy regarding code changes made by AI? Is it prohibited or accepted with chat history?

Chao Yang
This repo contains code for crystal lang's compiler which is also an interpreter. The interpreter should behave the same as executing the compiled binary. Read and modify relevant code files to fix an error that only happens when interpreting a program while working fine with compiled binary. Here's the stack trace when interpreting the program file: $ cr i local/abstract_method_bug.cr Using compiled compiler at .build/crystal Crystal interpreter 1.18.2 [635ca37a6] (2025-10-21). EXPERIMENTAL SOFTWARE: if you find a bug, please consider opening an issue in https://github.com/crystal-lang/crystal/issues/new/ Debug: ivar_index is nil: type=A+, name=@test Nil assertion failed (NilAssertionError) from src/nil.cr:113:7 in 'not_nil!' from src/nil.cr:109:3 in 'not_nil!' from src/compiler/crystal/interpreter/context.cr:398:18 in 'ivar_offset' from src/compiler/crystal/interpreter/compiler.cr:3500:7 in 'ivar_offset' from src/compiler/crystal/interpreter/compiler.cr:1038:21 in 'compile_instance_var' from src/compiler/crystal/interpreter/compiler.cr:1862:11 in 'visit' from src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept' from src/compiler/crystal/interpreter/compiler.cr:243:5 in 'compile_def' from src/compiler/crystal/interpreter/compiler.cr:2174:7 in 'create_compiled_def' from src/compiler/crystal/interpreter/compiler.cr:1908:22 in 'visit' from src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept' from src/compiler/crystal/interpreter/compiler.cr:3291:5 in 'accept_with_wants_value' from src/compiler/crystal/interpreter/compiler.cr:3281:5 in 'request_value' from src/compiler/crystal/interpreter/compiler.cr:2463:5 in 'compile_call_arg' from src/compiler/crystal/interpreter/compiler.cr:2391:7 in 'compile_call_args' from src/compiler/crystal/interpreter/compiler.cr:1904:5 in 'visit' from src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept' from src/compiler/crystal/interpreter/compiler.cr:589:7 in 'visit' from src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept' from src/compiler/crystal/interpreter/compiler.cr:243:5 in 'compile_def:closure_owner' from src/compiler/crystal/interpreter/compiler.cr:3245:5 in 'visit' from src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept' from src/compiler/crystal/interpreter/compiler.cr:589:7 in 'visit' from src/compiler/crystal/syntax/visitor.cr:27:12 in 'accept' from src/compiler/crystal/interpreter/compiler.cr:158:5 in 'compile' from src/compiler/crystal/interpreter/interpreter.cr:232:5 in 'interpret' from src/compiler/crystal/interpreter/repl.cr:94:5 in 'interpret' from src/compiler/crystal/interpreter/repl.cr:98:5 in 'interpret_and_exit_on_error' from src/compiler/crystal/interpreter/repl.cr:67:5 in 'run_file' from src/compiler/crystal/command/repl.cr:47:7 in 'repl' from src/compiler/crystal/command.cr:105:7 in 'run' from src/compiler/crystal/command.cr:56:5 in 'run' from src/compiler/crystal/command.cr:55:3 in 'run' from src/compiler/crystal.cr:11:1 in '__crystal_main' from src/crystal/main.cr:129:5 in 'main_user_code' from src/crystal/main.cr:115:7 in 'main' from src/crystal/system/unix/main.cr:9:3 in 'main' from /lib/x86_64-linux-gnu/libc.so.6 in '??' from /lib/x86_64-linux-gnu/libc.so.6 in '__libc_start_main' from /home/chao/git/personal/crystal/.build/crystal in '_start' from ???

---

omitted chain of thoughts

The issue was that when inlining a method that returns an instance variable (like B#foo returning @test), the interpreter was using the scope of the calling method (A#bar) instead of the scope of the method being inlined (B#foo). This caused the interpreter to look for the instance variable @test in class A, where it doesn't exist, leading to the error.

I've corrected this by modifying src/compiler/crystal/interpreter/compiler.cr to ensure the correct scope is used during inlining. Specifically:

I updated the compile_instance_var method to accept an owner type, which specifies the scope in which to look up the instance variable.
In the visit(Call) method, when an instance variable access is inlined, I now pass the correct owner type (target_def.owner) to compile_instance_var.
These changes should resolve the bug.

[straight-shoota]

For me, it's not relevant how a patch came together. If you rolled a dice, or trained a monkey to do it, that's all fine as long as you have the rights to license the code under our license terms.

The contribution guide does not have any specific rules regarding AI use.

All the general rules apply, of course. For example: describe reasons and result of the change in the pull request comment.
A proper description of the reasons should look something like @HertzDevil's #16303 (comment) or @ysbaddaden's #16307 (review). But it should not be the reviewers' responsibility to investigate how a bugfix works.
Giving no reasoning is insufficient, and nobody wants to digest AI chat log in order to maybe gain some understanding.
Pull requests should provide a concise description for any non-trivial changes.

[cyangle]

OK, so it's better to provide summary of AI analysis to the github issue first before creating a PR. And it looks like the AI is helpful at finding root causes of bugs.

[ysbaddaden]

@cyangle There is no need to keep merging master into your branches.