I'm following along with the book, Crafting Interpreters, but I'm implementing jlox in C# instead, as I thought it would help me understand the code better if I had to do some amount of porting. I am running into language specific issues though, particularly, I believe, with how C# handles null.

After implementing the resolver from chapter 11, my interpreter can no longer handle for loops (and probably while loops too, though I didn't test this).

The resolver does not seem to properly store the location of the variable declaration in the for loop in its dictionary. This leads to it passing the wrong 'distance' to the interpreter.

Here's an example:

for (var i = 0; i < 10; i = i + 1) {
    print i;
}

My interpreter will crash with this code that should be valid, because when it encounters print i it won't be able to find the declaration for i. The next chapter of the book is about classes, so this error is not supposed to be here.

I apologize for the complexity of the code and the low amount of comments, my plan was to complete this part of the book and then reread it and go over and comment all my code so I can understand it better. I've attached a link to the GitHub issue on this post, it has a simple explanation and the stack-trace .NET throws when it encounters a for loop.

Please let me know if you need more information.

I won't be surprised if it turns out to be a very simple mistake, but I will facepalm.

you are viewing a single comment's thread
view the rest of the comments

[–] dr_robotBones@reddthat.com 2 points 3 weeks ago (1 children)

Wow I didn't know. Thank you for pointing that out.

I know in C# you can just pull from data structures using an index, so can I do stack[0]? Would that be the same as Java's stack.elementAt(0)?

[–] xianjam@programming.dev 3 points 3 weeks ago* (last edited 3 weeks ago) (1 children)

Stack doesn't actually allow indexing, and ElementAt is an extension method implemented by accessing Stack's iterator. My honest recommendation is to not use Stack, even if that's what the book was using. Stacks are not really meant to be traversed. They're meant to be pushed to, and popped (or peeked) from. Each ElementAt call requires O(N) time to retrieve. I recommend switching to List and implementing 'push', 'pop', and 'peek' as such:

// Instead of Push
scopes.Add(item);

// Instead of Peek
int peeked = scopes[^1];

// Instead of Pop
int popped = scopes[^1];
scopes.RemoveAt(scopes.Count - 1);

However, I understand that is a bit of a change. If you'd prefer to keep it as a stack, a simple reversal of the for loop can get the job done.

	// Resolve local variable assignment
	private void ResolveLocal(Expr expr, Token name) {
		// Decrement through scopes
		for (int i = 0; i < scopes.Count; i++) {
			// Pass amount of layers since declaration to interpreter
			if (scopes.ElementAt(i).ContainsKey(name.getLexeme())) {
				interpreter.Resolve(expr, i);
				return;
			}
		}
	}

That's enough to get these scripts running.

>for(var i = 0; i < 10; i = i + 1) { print i;} 
0
1
2
3
4
5
6
7
8
9

>var a = 0; var temp; for(var b= 1; a < 10000; b = temp + b) { print a; temp = a; a = b;}
0
1
1
2
3
5
8
13
21
34
55
89
144
233
377
610
987
1597
2584
4181
6765

By the way, I noticed that redefining a global variable will crash as well, but a scoped variable will display an error as intended.

[–] dr_robotBones@reddthat.com 2 points 3 weeks ago

Thanks for bringing that global variable issue to my attention, I'll have to look into it.

As for the stack, I'll traverse it in reverse for now, I may come back once I've finished the interpreter to clean up the code and add more features.