emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.74k stars 3.3k forks source link

enum values within namespaces can overwrite each other when using WebIDL #13243

Open fabmax opened 3 years ago

fabmax commented 3 years ago

I guess this is a bit of a corner case. Following situation:

C++ code:

namespace NameSpaceA {
    enum EnumA {
        SomeValue = 1
    };
};
namespace NameSpaceB {
    enum EnumB {
        SomeValue = 17
    };
};
// use typedefs so enums in namespaces can be mapped in WebIDL
typedef NameSpaceA::EnumA NameSpaceA_EnumA;
typedef NameSpaceB::EnumB NameSpaceB_EnumB;

Corresponding idl file:

enum NameSpaceA_EnumA {
    "NameSpaceA_EnumA::SomeValue"
};
enum NameSpaceB_EnumB {
    "NameSpaceB_EnumB::SomeValue"
};

This code compiles. However the problem is that, in javascript, the enum values are accessed by their unquallified name. Hence there is only a single Module.SomeValue defined and one of the two values gets lost.

I'm currently mapping nvidia PhysX with WebIDL and such enums are used as flags all over the place. Passing wrong values here can result in pretty weird and unexpected behavior. My current work-around is to use the internal functions generated by WebIDL Binder instead (e.g. Module._emscripten_enum_NameSpaceA_EnumA_SomeValue()).

However I think it would be better to map enum values with their quallified name or at least issue some kind of warning since this can produce bugs which are quite hard to track down.

kripken commented 3 years ago

Yes, this does seem confusing...

Perhaps reporting an error for this would be enough. Then it can be worked around in the source.

I'm not strongly opposed to prefixing the names. That would be a breaking change, however, so we'd need to think about it more carefully.

A good starting point would be if someone has time to investigate how practical those two options are.

fabmax commented 3 years ago

I'm not sure if this is possible or if it would violate the WebIDL standard somehow but I think the best option would be to introduce an optional decorator. E.g. something like this:

[JSPrefix="xyz"]
enum NameSpaceA_EnumA {
    "NameSpaceA_EnumA::SomeValue"
};

This way the normal behavior could stay the same without breaking anything and, if the decorator is present, the javascript enum values would be generated with the specified prefix (xyz_SomeValue in this case)

kripken commented 3 years ago

That's an option too, we've been adding custom decorators as necessary.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because there has been no activity in the past year. It will be closed automatically if no further activity occurs in the next 30 days. Feel free to re-open at any time if this issue is still relevant.

regnaio commented 1 month ago

I've been using pmndrs/webidl-dts-gen to generate types for fabmax/physx-js-webidl by:

npx webidl-dts-gen -e -n PhysX -i PhysX/physx/source/webidlbindings/src/wasm/PhysXWasm.idl -o dist/physx-js-webidl.wasm.d.ts

It returns warnings like:

Duplicate enum member name: 'eRIGID_STATIC'. Omitting duplicate from types. Enums in emscripten are exposed on the module their names, e.g. 'Module.MemberName', not 'Module.Enum.MemberName'

Although there is a workaround on the JavaScript side (https://github.com/fabmax/PhysX/pull/1) to allow for Module.Enum.MemberName, on the TypeScript side, I still have to use Module.MemberName instead of Module.Enum.MemberName