Jordan Rome

Results 346 comments of Jordan Rome

Thanks for the reviews @viktormalik @danobi @ajor ! Going to merge shortly 🚀

> I'm thinking that it might make more sense for a loop to "have a" Scope, rather than for it to be an "is a" relationship. Yeah, I like this....

``` visit(Scope &scope) { scope_stack_.push_back(scope); for (Statement &stmt : scope.stmts) { accept(stmt); } scope_stack_.pop_back(); } ``` Looking again at the individual places where we push on to the scope_stack, there...

> I don't think there's any need for the warning and loop condition to occur within the scope of the loop's body. I think the condition *should* exist within the...

> Instead of iterating over the entire scope stack each time we need to look up a variable, could we keep a cached set of valid variable names for the...

> When we come to the print($d) statement, the current implementation will iterate over each level of the scope stack and perform a map-lookup in each one until it finds...

> Agreed about for-loops, and that's why I think Scope is a misleading name for this node! Still not sure if I understand why the name is misleading for this...

@ajor and I cleared some things up offline. Going to put up a few minor changes and then I think this should be good to go.

Please vote here if you want this diff to move forward. *Note*: we will first tally the votes from only maintainers and very active contributors but also take into account...

@viktormalik I think I addressed all your comments. 🤞