This is an archive of the discontinued LLVM Phabricator instance.

[WebAssembly] Add an import_field function attribute
ClosedPublic

Authored by sunfish on Feb 1 2019, 10:32 AM.

Details

Summary

This is similar to import_module, but sets the import field name instead.

This is somewhat subtle. By default, the import field name is the same as the C/asm/.o symbol name. However, there are situations where it's useful to have it be different. For example, suppose I have a wasm API with a module named "pwsix" and a field named "read". There's no risk of namespace collisions with user code at the wasm level because the generic name "read" is qualified by the module name "pwsix". However in the C/asm/.o namespaces, the module name is not used, so if I have a global function named "read", it is intruding on the user's namespace.

With the import_field module, I can declare my function (in libc) to be "__read", and then set the wasm import module to be "pwsix" and the wasm import field to be "read". So at the C/asm/.o levels, my symbol is outside the user namespace.

Diff Detail

Repository
rC Clang

Event Timeline

sunfish created this revision.Feb 1 2019, 10:32 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 10:32 AM
sbc100 accepted this revision.Feb 1 2019, 11:39 AM

Do you could also achieve this I guess by renaming pwsix.read function itself to pwsix.__read? But I guess it makes sense to allow the use to control all 3 parts of the name separately.

BTW, I've never liked the word field here. I think "wasm-import-name" would make more sense, but that would conflict with the spec I suppose, and also this should be very rarely needed attr.

This revision is now accepted and ready to land.Feb 1 2019, 11:39 AM

Do you could also achieve this I guess by renaming pwsix.read function itself to pwsix.__read? But I guess it makes sense to allow the use to control all 3 parts of the name separately.

Yeah. As another example, if I wanted to import "print_i32" from "spectest", I might want C to call it "spectest_print_i32" or something, and not be forced to have it occupy "print_i32" in C's global namespace.

BTW, I've never liked the word field here. I think "wasm-import-name" would make more sense, but that would conflict with the spec I suppose, and also this should be very rarely needed attr.

I just checked the spec and it does't actually use the word "field" here. So I'll rename this attribute to "import_name" and "wasm-import-name".

great. lgtm

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 1 2019, 2:25 PM