Skip to content

Conversation

ffrostfall
Copy link
Contributor

This PR implements an optimization where a shaped table is cloned, instead of an empty completely dynamic table.

This also required the implementation of upvalues into codegen & LIR

Copy link
Owner

@1Axen 1Axen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have some concerns that I've left as comments, otherwise looks good!


builder:append(BUFFER_TEMPLATE)

builder:append("-- upvalues start\n")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
builder:append("-- upvalues start\n")
builder:append_line("-- upvalues start")

for upval_id, default in lirr.upvalues do
gen_upvalue(context, upval_id, default)
end
builder:append("-- upvalues end\n")
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
builder:append("-- upvalues end\n")
builder:append_line("-- upvalues end")

Comment on lines 109 to 117
export type ProjShapedTable<T> = {
write __phantom_type: T,

type: "projection",
kind: "shaped_table",
base: LValue<T>,
shape: { string },
}

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you forget to remove this?

Comment on lines +213 to +218
local function add_upvalue<T>(self: Builder, upvalue_name: string, upvalue_default: RValue<T>)
local upval = upvalue(upvalue_name)
self.ctx.lir.upvalues[upval] = upvalue_default

return upval
end
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have a panic here for duplicate upvalues entries

Comment on lines +93 to +100
local ty = hir.ty_from_id(ctx.hir, decl.ty_id)

if ty.kind == "struct" then
local template = create_struct_template(decl.ty_id, ty)

ctx.lir.upvalues[builders.fn.upvalue(tostring(decl.ty_id))] = template
end

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would need to build these in a different place (builders/ty possibly?) or we'd have to copy this segment for events and functs too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants