Skip to content

Support declaring anonymous functions and data objects#2750

Merged
cfallin merged 1 commit intobytecodealliance:mainfrom
bjorn3:anon_allocs
Apr 12, 2021
Merged

Support declaring anonymous functions and data objects#2750
cfallin merged 1 commit intobytecodealliance:mainfrom
bjorn3:anon_allocs

Conversation

@bjorn3
Copy link
Contributor

@bjorn3 bjorn3 commented Mar 21, 2021

Fixes #2220

@github-actions github-actions bot added cranelift Issues related to the Cranelift code generator cranelift:module labels Mar 21, 2021
@bjorn3
Copy link
Contributor Author

bjorn3 commented Apr 7, 2021

@pchickey Could you please review this PR?

Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

This looks reasonable to me overall; the API changes are relatively minor. Thanks!

It seems a little weird to rely on symbol naming to communicate to the linker to elide certain symbols from the symbol table; I guess there isn't a better out-of-band way (symbol type, etc) to ensure this happens?

Happy to merge if we're satisfied there's not a better way re: above!

@bjorn3
Copy link
Contributor Author

bjorn3 commented Apr 12, 2021

Not that I know of. Using .L as prefix is the standard way to indicate labels. Even LLVM uses symbol names like .L__unnamed_1.

@cfallin
Copy link
Member

cfallin commented Apr 12, 2021

Sounds good, then; thanks for looking into it!

@cfallin cfallin merged commit 67cc42d into bytecodealliance:main Apr 12, 2021
@bjorn3 bjorn3 deleted the anon_allocs branch April 12, 2021 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cranelift:module cranelift Issues related to the Cranelift code generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add define_anonymous_data function to cranelift-module

2 participants