-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Description
This is a follow-up to issue #1201, which was originally submitted as a bug report. After further reflection and your helpful feedback, I now see that it's not a user-visible bug.
That said, I would like to reframe this as an internal safety and cleanup suggestion.
Currently, the compiler assigns "this" to slot 0 unconditionally, even in non-method functions. While this is harmless under normal conditions due to compile-time checks, it can make the compiler harder to reason about, especially when extending the language or modifying internals.
A conditional check for currentClass before assigning "this" could improve clarity and reduce hidden assumptions.
Thanks for considering!
A More Detailed Description of This Bug
Once again, you’re avoiding the essence of the issue and just rambling nonsense.
I clearly told you to look at the code, didn’t I?
From the way you keep talking without even checking it, it’s obvious you haven’t read what I wrote at all.
Let’s be civilized . I’ll show you the actual code.
The author did consider where this can and cannot be used.
Line 329 of compile.c:
if (type != TYPE_FUNCTION) {
local->name.start = "this";
local->name.length = 4;
} else {
local->name.start = "";
local->name.length = 0;
}Now, do you see this?
When does this get added? — when type != TYPE_FUNCTION, right?
And when is type not TYPE_FUNCTION? You don’t know, because you haven’t actually read the book, have you?
When type is not TYPE_FUNCTION, it means we’re in either the global scope or the class scope.
I’m almost certain the author failed to properly handle the global scope here.
The correct implementation should look like this:
if (currentClass != null || type != TYPE_FUNCTION) {
local->name.start = "this";
local->name.length = 4;
} else {
local->name.start = "";
local->name.length = 0;
}this should only ever be stored within class scope.
As a result, since the author missed this case, we later see in line 823 of compiler.c:
static void this_(bool canAssign) {
//> this-outside-class
if (currentClass == NULL) {
error("Can't use 'this' outside of a class.");
return;
}
//< this-outside-class
variable(false);
} // [this]The author had to manually add an extra check for currentClass == NULL.
But if the code had been written correctly like in my fixed version , there would be no need for this redundant logic to prevent this from being stored in the global scope in the first place.
In the end, the author just added a workaround to cover the mistake they overlooked. Doesn’t the approach I suggested look cleaner? Do you still think I’m being unreasonable?
Now do you understand?
Student? Maybe read a Software Methodology textbook before your next compiler class instead of spouting nonsense.
If you’re a real developer, speak through code, not empty words.
New evidence of a bug
I am writing this new post to present additional evidence supporting my claim.
For those who are not familiar with the previous discussion, please read the following old post first.
OpsikionThemed mentioned that:
“I checked the book and Nystrom specifically explains how he’s putting an extra stack slot into every function for exactly the purpose of making implementing this simpler and more uniform.__”
However, if you look at the actual code (compiler.c), you can see that the this pointer is stored only when type is not TYPE_FUNCTION.
In other words, the author designed the logic so that the this pointer is stored only when the context is not a regular function which includes both the global context and the class context.
Therefore, I am convinced that the author failed to additionally account for the global context, and I believe it is correct to classify this as a bug. I would be very interested to hear any counterarguments to this reasoning.
Lastly, I would like to ask for the support of honest engineers who value technical integrity. Please help ensure that those who do the right thing are not unfairly criticized for it.
a misunderstanding about a bug in an interpreter
I’m the person who recently posted about finding a bug in Crafting Interpreters. That post has since been deleted by Reddit. I’d like to clearly restate the issue here once again, as there have been many misunderstandings. Hopefully, this will put an end to any further controversy.
The bug I discovered is as follows:
- The this pointer is always stored in slot 0 of the global stack. However, since the global scope is not a class context, it should not have a this pointer in the stack.
The main misunderstanding is that some people believe this issue occurred because I removed the access restriction that prevents using this in the global scope. That’s not true. I only lifted that restriction to demonstrate that the this pointer is indeed stored in the global scope. Logically speaking, the global scope should never contain a this pointer. Therefore, even if you remove the access restriction, there should be no this pointer to access, and attempting to do so should result in a runtime error. However, since the this pointer actually exists in the global scope’s stack, it can be accessed without any runtime error—which is the core of the problem.
Let me address a few common counterarguments:
-
“This bug occurred because you removed the restriction on using
thisin the global scope.” → No. The fact that the this pointer is stored in slot 0 of the global stack has nothing to do with that restriction. I removed the restriction precisely to demonstrate that the global stack does indeed contain a this pointer. Once again, the global scope should not contain a this pointer at all. The fact that it does—and that removing the restriction allows smooth access to it—proves the bug. To repeat: there should be no this pointer in slot 0 of the global stack, and any attempt to access it should result in a runtime error. -
“Use of
thisin the global scope is already blocked at compile time.” → Yes, that’s true. But the this pointer is still stored in slot 0 of the global stack. -
“Even if the
thispointer remains in the global scope, it’s not a real problem.” → I disagree. This violates the logical and semantic consistency of the language. Crafting Interpreters is an excellent book read by many beginners, and it would be best not to leave such inconsistencies in it. Moreover, although unlikely, this behavior could potentially be exploited by attackers for memory-related vulnerabilities. While the risk is small, no one can say it’s impossible. It’s better to eliminate such issues proactively, rather than acknowledging and patching them only after they’re exploited.
In summary: → The this pointer is always stored in slot 0 of the global stack, even though the global scope is not a class context. It should not exist there.
I consider this a serious issue and therefore reported it as a bug. However, people who agree with the author have ignored the core problem and instead accused me of causing the issue by modifying the code to allow access to this.
I believe there are many honest engineers like me who won’t join in this kind of biased criticism. Please judge this matter objectively—not by the number of comments or upvotes, but by carefully reading and understanding what’s actually being said.
Here are the related issue reports and pull requests:
I trust that other sincere engineers will stand by me. Please help ensure that those who do the right thing are not unfairly criticized again.
Thanks to OpsikionThemed, I found new evidence of this bug.
