emscripten-core / emscripten

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

Documentation doesn't match implementation in WebIDL_Binder: Use of [Const] attribute for methods #14747

Open ArthurG0 opened 3 years ago

ArthurG0 commented 3 years ago

Hello,

Noticed a mismatch between documentation and implementation related to the use of [Const] attributes for method return types in the tools/webidl_binder.py

From the documentation:

C++ arguments or return types that use const can be specified in IDL using [Const].

For example, the following code fragments show the C++ and IDL for a function that returns a constant pointer object.

//C++ const myObject* getAsConst(); // WebIDL [Const] myObject getAsConst();

So the documentation states that [Const] attribute applies to the return type of the method.

However, in reality, using the [Const] attribute in the manner above results in the method being marked as const, not the return type. This is particularly a problem when generating the Javascript implementation class for an abstract C++ interface.

Example:

[Const] DOMString foo();

turns into

char* foo() const in the generated Glue C++ source file.

The documentation never mentions that const methods are supported, so I believe this is a bug in the implementation.

ArthurG0 commented 3 years ago

@sbc100

sbc100 commented 3 years ago

I'm not too familiar with webIDL binder myself. Would you be interesting if uploading a tests case (that shows how the resulting const on the method is an issue)? and maybe a patch for the implementation?

agnickolov commented 3 years ago

We did an internal investigation and the problem turned out to be due to a poorly designed grammar in the WebIDL standard itself: https://www.w3.org/TR/WebIDL-1/ The issue is that a method declaration swallows all extended attributes and applies them to the method itself, thus the observed behavior: https://www.w3.org/TR/WebIDL-1/#prod-InterfaceMembers InterfaceMembers : ExtendedAttributeList InterfaceMember InterfaceMembers This makes it impossible to attach extended attributes to the return type. One possible workaround hack would be to patch the behavior within the binder and apply the method extended attributes to the return type instead, essentially ignoring what the AST says. Of course, this would mean there would be no way to declare a const method in an interface... A different way to solve the problem is to map DOMString to const char instead of char thus obviating the need for a [Const] attribute. I'm not sure why this hasn't been done from the get go, the prescribed behavior is ridiculous to any seasoned C++ developer... Perhaps this can be implemented as behavior controlled with a command-line switch so as not to break existing code. At the very least, the documentation on the emscripten web site should be updated to reflect what the WebIDL binder actually does.