Stage2: Implement comptime closures and the This builtin #9823
Conversation
src/Module.zig
Outdated
| /// child decls. Values stored in the value_arena of the linked decl. | ||
| /// During sema, this map is backed by the gpa. Once sema completes, | ||
| /// it is reallocated using the value_arena. | ||
| captures: std.AutoArrayHashMapUnmanaged(Zir.Inst.Ref, TypedValue) = .{}, |
There was a problem hiding this comment.
As far as I can tell, whenever a key is added here it is always an Index, not a Ref, and then indexToRef is used on the way in, and refToIndex on the way out. Simpler would be to make the key an Index right? And then those functions don't need to be moved from AstGen.
There was a problem hiding this comment.
I tried that originally, but it doesn't quite work. zirClosureGet uses the un_node field, which uses Ref as its operand. I could reinterpret it as an index, but that would be really nasty. The indexToRef in zirClosureCapture could be moved to be a refToIndex in zirClosureGet, but it doesn't really improve the situation much. A second option would be to introduce a un_node_index field, but that also feels like overkill to me.
There was a problem hiding this comment.
I don't think it's overkill - plenty of ZIR instructions have their own form. closure_get doesn't want a un_node, it wants a node and a zir index, right? Let's give it the type-correct thing that it wants.
Another argument is that it helps readability/maintainability of the codebase. Requiring a Ref or an Index mean different things and it could be confusing to someone reading the code that it requires the wrong one.
| // use a temp to avoid unintentional aliasing due to RLS | ||
| const tmp = try self.scope.captures.clone(self.perm_arena); | ||
| self.scope.captures = tmp; |
There was a problem hiding this comment.
sound like we need to have a big language design discussion
There was a problem hiding this comment.
I'm always down for that! This is #3696 and friends.
SpexGuy
force-pushed
the
comptime-closures
branch
from
c7ec3a0 to
5e20b5b
Compare
No description provided.