Skip to content

Commit 2c44617

Browse files
brendandahlbrendandahl
authored
[embind] Support annotating return values as nonnull. (emscripten-core#22567)
Embind's subclass `implement` methods were generated as returning `Class | null` after the changes to pointer types in emscripten-core#22184. This could be considered a regression as the implement method would never return null. Previously, we had special handling so constructors were marked as nonnull so in the TS definitions we didn't add `| null`. I've generalized this approach to work for all function bindings so they can now use a `nonnull<ret_val>` policy too avoid the `| null`.
1 parent 816497f commit 2c44617

File tree

12 files changed

+173
-32
lines changed

12 files changed

+173
-32
lines changed

‎ChangeLog.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ See docs/process.md for more on how version tagging works.
2020

2121
3.1.67 (in development)
2222
-----------------------
23+
- Add option `nonnull<ret_val>()` to Embind to omit `| null` from TS definitions
24+
for functions that return pointers.
2325

2426
3.1.66 - 09/10/24
2527
-----------------

‎site/source/docs/porting/connecting_cpp_and_javascript/embind.rst‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1211,6 +1211,16 @@ registered using :cpp:func:`EMSCRIPTEN_DECLARE_VAL_TYPE` in combination with
12111211
register_type<CallbackType>("(message: string) => void");
12121212
}
12131213
1214+
1215+
``nonnull`` Pointers
1216+
--------------------
1217+
1218+
C++ functions that return pointers generate TS definitions with ``<SomeClass> |
1219+
null`` to allow ``nullptr`` by default. If the C++ function is guaranteed to
1220+
return a valid object, then a policy parameter of ``nonnull<ret_val>()`` can be
1221+
added to the function binding to omit ``| null`` from TS. This avoids having to
1222+
handle the ``null`` case in TS.
1223+
12141224
Performance
12151225
===========
12161226

‎src/embind/embind.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -951,7 +951,7 @@ var LibraryEmbind = {
951951
'$craftInvokerFunction', '$exposePublicSymbol', '$heap32VectorToArray',
952952
'$readLatin1String', '$replacePublicSymbol', '$embind__requireFunction',
953953
'$throwUnboundTypeError', '$whenDependentTypesAreResolved', '$getFunctionName'],
954-
_embind_register_function: (name, argCount, rawArgTypesAddr, signature, rawInvoker, fn, isAsync) => {
954+
_embind_register_function: (name, argCount, rawArgTypesAddr, signature, rawInvoker, fn, isAsync, isNonnullReturn) => {
955955
var argTypes = heap32VectorToArray(argCount, rawArgTypesAddr);
956956
name = readLatin1String(name);
957957
name = getFunctionName(name);
@@ -1941,7 +1941,8 @@ var LibraryEmbind = {
19411941
rawInvoker,
19421942
context,
19431943
isPureVirtual,
1944-
isAsync) => {
1944+
isAsync,
1945+
isNonnullReturn) => {
19451946
var rawArgTypes = heap32VectorToArray(argCount, rawArgTypesAddr);
19461947
methodName = readLatin1String(methodName);
19471948
methodName = getFunctionName(methodName);
@@ -2077,7 +2078,8 @@ var LibraryEmbind = {
20772078
invokerSignature,
20782079
rawInvoker,
20792080
fn,
2080-
isAsync) => {
2081+
isAsync,
2082+
isNonnullReturn) => {
20812083
var rawArgTypes = heap32VectorToArray(argCount, rawArgTypesAddr);
20822084
methodName = readLatin1String(methodName);
20832085
methodName = getFunctionName(methodName);

‎src/embind/embind_gen.js‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,13 @@ var LibraryEmbind = {
4444
},
4545
$FunctionDefinition__deps: ['$createJsInvoker', '$createJsInvokerSignature', '$emittedFunctions'],
4646
$FunctionDefinition: class {
47-
constructor(name, returnType, argumentTypes, functionIndex, thisType = null, isConstructor = false, isAsync = false) {
47+
constructor(name, returnType, argumentTypes, functionIndex, thisType = null, isNonnullReturn = false, isAsync = false) {
4848
this.name = name;
4949
this.returnType = returnType;
5050
this.argumentTypes = argumentTypes;
5151
this.functionIndex = functionIndex;
5252
this.thisType = thisType;
53-
this.isConstructor = isConstructor;
53+
this.isNonnullReturn = isNonnullReturn;
5454
this.isAsync = isAsync;
5555
}
5656

@@ -80,7 +80,7 @@ var LibraryEmbind = {
8080
// Constructors can return a pointer, but it will be a non-null pointer.
8181
// Change the return type to the class type so the TS output doesn't
8282
// have `| null`.
83-
if (this.isConstructor && this.returnType instanceof PointerDefinition) {
83+
if (this.isNonnullReturn && this.returnType instanceof PointerDefinition) {
8484
returnType = this.returnType.classType;
8585
}
8686
out.push(`): ${nameMap(returnType, true)}`);
@@ -456,7 +456,7 @@ var LibraryEmbind = {
456456
registerType(id, new IntegerType(id));
457457
},
458458
$createFunctionDefinition__deps: ['$FunctionDefinition', '$heap32VectorToArray', '$readLatin1String', '$Argument', '$whenDependentTypesAreResolved', '$getFunctionName', '$getFunctionArgsName', '$PointerDefinition', '$ClassDefinition'],
459-
$createFunctionDefinition: (name, argCount, rawArgTypesAddr, functionIndex, hasThis, isConstructor, isAsync, cb) => {
459+
$createFunctionDefinition: (name, argCount, rawArgTypesAddr, functionIndex, hasThis, isNonnullReturn, isAsync, cb) => {
460460
const argTypes = heap32VectorToArray(argCount, rawArgTypesAddr);
461461
name = typeof name === 'string' ? name : readLatin1String(name);
462462

@@ -486,7 +486,7 @@ var LibraryEmbind = {
486486
args.push(new Argument(`_${i - argStart}`, argTypes[i]));
487487
}
488488
}
489-
const funcDef = new FunctionDefinition(name, returnType, args, functionIndex, thisType, isConstructor, isAsync);
489+
const funcDef = new FunctionDefinition(name, returnType, args, functionIndex, thisType, isNonnullReturn, isAsync);
490490
cb(funcDef);
491491
return [];
492492
});
@@ -537,8 +537,8 @@ var LibraryEmbind = {
537537
// TODO
538538
},
539539
_embind_register_function__deps: ['$moduleDefinitions', '$createFunctionDefinition'],
540-
_embind_register_function: (name, argCount, rawArgTypesAddr, signature, rawInvoker, fn, isAsync) => {
541-
createFunctionDefinition(name, argCount, rawArgTypesAddr, fn, false, false, isAsync, (funcDef) => {
540+
_embind_register_function: (name, argCount, rawArgTypesAddr, signature, rawInvoker, fn, isAsync, isNonnullReturn) => {
541+
createFunctionDefinition(name, argCount, rawArgTypesAddr, fn, false, isNonnullReturn, isAsync, (funcDef) => {
542542
moduleDefinitions.push(funcDef);
543543
});
544544
},
@@ -598,8 +598,9 @@ var LibraryEmbind = {
598598
rawInvoker,
599599
context,
600600
isPureVirtual,
601-
isAsync) {
602-
createFunctionDefinition(methodName, argCount, rawArgTypesAddr, context, true, false, isAsync, (funcDef) => {
601+
isAsync,
602+
isNonnullReturn) {
603+
createFunctionDefinition(methodName, argCount, rawArgTypesAddr, context, true, isNonnullReturn, isAsync, (funcDef) => {
603604
const classDef = funcDef.thisType;
604605
classDef.methods.push(funcDef);
605606
});
@@ -637,10 +638,11 @@ var LibraryEmbind = {
637638
invokerSignature,
638639
rawInvoker,
639640
fn,
640-
isAsync) {
641+
isAsync,
642+
isNonnullReturn) {
641643
whenDependentTypesAreResolved([], [rawClassType], function(classType) {
642644
classType = classType[0];
643-
createFunctionDefinition(methodName, argCount, rawArgTypesAddr, fn, false, false, isAsync, (funcDef) => {
645+
createFunctionDefinition(methodName, argCount, rawArgTypesAddr, fn, false, isNonnullReturn, isAsync, (funcDef) => {
644646
classType.staticMethods.push(funcDef);
645647
});
646648
return [];

‎src/library_sigs.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -288,17 +288,17 @@ sigs = {
288288
_embind_register_bigint__sig: 'vpppjj',
289289
_embind_register_bool__sig: 'vppii',
290290
_embind_register_class__sig: 'vppppppppppppp',
291-
_embind_register_class_class_function__sig: 'vppippppi',
291+
_embind_register_class_class_function__sig: 'vppippppii',
292292
_embind_register_class_class_property__sig: 'vpppppppp',
293293
_embind_register_class_constructor__sig: 'vpipppp',
294-
_embind_register_class_function__sig: 'vppippppii',
294+
_embind_register_class_function__sig: 'vppippppiii',
295295
_embind_register_class_property__sig: 'vpppppppppp',
296296
_embind_register_constant__sig: 'vppd',
297297
_embind_register_emval__sig: 'vp',
298298
_embind_register_enum__sig: 'vpppi',
299299
_embind_register_enum_value__sig: 'vppi',
300300
_embind_register_float__sig: 'vppp',
301-
_embind_register_function__sig: 'vpippppi',
301+
_embind_register_function__sig: 'vpippppii',
302302
_embind_register_integer__sig: 'vpppii',
303303
_embind_register_memory_view__sig: 'vpip',
304304
_embind_register_optional__sig: 'vpp',

‎system/include/emscripten/bind.h‎

Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,8 @@ void _embind_register_function(
105105
const char* signature,
106106
GenericFunction invoker,
107107
GenericFunction function,
108-
bool isAsync);
108+
bool isAsync,
109+
bool isNonnullReturn);
109110

110111
void _embind_register_value_array(
111112
TYPEID tupleType,
@@ -182,7 +183,8 @@ void _embind_register_class_function(
182183
GenericFunction invoker,
183184
void* context,
184185
unsigned isPureVirtual,
185-
bool isAsync);
186+
bool isAsync,
187+
bool isNonnullReturn);
186188

187189
void _embind_register_class_property(
188190
TYPEID classType,
@@ -204,7 +206,8 @@ void _embind_register_class_class_function(
204206
const char* invokerSignature,
205207
GenericFunction invoker,
206208
GenericFunction method,
207-
bool isAsync);
209+
bool isAsync,
210+
bool isNonnullReturn);
208211

209212
void _embind_register_class_class_property(
210213
TYPEID classType,
@@ -338,6 +341,15 @@ struct pure_virtual {
338341
};
339342
};
340343

344+
template<typename Slot>
345+
struct nonnull {
346+
static_assert(std::is_same<Slot, ret_val>::value, "Only nonnull return values are currently supported.");
347+
template<typename InputType, int Index>
348+
struct Transform {
349+
typedef InputType type;
350+
};
351+
};
352+
341353
namespace return_value_policy {
342354

343355
struct take_ownership : public allow_raw_pointers {};
@@ -380,6 +392,11 @@ struct isPolicy<emscripten::pure_virtual, Rest...> {
380392
static constexpr bool value = true;
381393
};
382394

395+
template<typename T, typename... Rest>
396+
struct isPolicy<emscripten::nonnull<T>, Rest...> {
397+
static constexpr bool value = true;
398+
};
399+
383400
template<typename T, typename... Rest>
384401
struct isPolicy<T, Rest...> {
385402
static constexpr bool value = isPolicy<Rest...>::value;
@@ -428,6 +445,24 @@ struct isAsync<> {
428445
static constexpr bool value = false;
429446
};
430447

448+
template<typename... Policies>
449+
struct isNonnullReturn;
450+
451+
template<typename... Rest>
452+
struct isNonnullReturn<nonnull<ret_val>, Rest...> {
453+
static constexpr bool value = true;
454+
};
455+
456+
template<typename T, typename... Rest>
457+
struct isNonnullReturn<T, Rest...> {
458+
static constexpr bool value = isNonnullReturn<Rest...>::value;
459+
};
460+
461+
template<>
462+
struct isNonnullReturn<> {
463+
static constexpr bool value = false;
464+
};
465+
431466
}
432467

433468
////////////////////////////////////////////////////////////////////////////////
@@ -640,7 +675,8 @@ void function(const char* name, ReturnType (*fn)(Args...), Policies...) {
640675
getSignature(invoke),
641676
reinterpret_cast<GenericFunction>(invoke),
642677
reinterpret_cast<GenericFunction>(fn),
643-
isAsync<Policies...>::value);
678+
isAsync<Policies...>::value,
679+
isNonnullReturn<Policies...>::value);
644680
}
645681

646682
namespace internal {
@@ -1516,7 +1552,8 @@ struct RegisterClassMethod<ReturnType (ClassType::*)(Args...)> {
15161552
reinterpret_cast<GenericFunction>(invoke),
15171553
getContext(memberFunction),
15181554
isPureVirtual<Policies...>::value,
1519-
isAsync<Policies...>::value);
1555+
isAsync<Policies...>::value,
1556+
isNonnullReturn<Policies...>::value);
15201557
}
15211558
};
15221559

@@ -1545,7 +1582,8 @@ struct RegisterClassMethod<ReturnType (ClassType::*)(Args...) const> {
15451582
reinterpret_cast<GenericFunction>(invoke),
15461583
getContext(memberFunction),
15471584
isPureVirtual<Policies...>::value,
1548-
isAsync<Policies...>::value);
1585+
isAsync<Policies...>::value,
1586+
isNonnullReturn<Policies...>::value);
15491587
}
15501588
};
15511589

@@ -1573,7 +1611,8 @@ struct RegisterClassMethod<ReturnType (*)(ThisType, Args...)> {
15731611
reinterpret_cast<GenericFunction>(invoke),
15741612
getContext(function),
15751613
false,
1576-
isAsync<Policies...>::value);
1614+
isAsync<Policies...>::value,
1615+
isNonnullReturn<Policies...>::value);
15771616
}
15781617
};
15791618

@@ -1601,7 +1640,8 @@ struct RegisterClassMethod<std::function<ReturnType (ThisType, Args...)>> {
16011640
reinterpret_cast<GenericFunction>(invoke),
16021641
getContext(function),
16031642
false,
1604-
isAsync<Policies...>::value);
1643+
isAsync<Policies...>::value,
1644+
isNonnullReturn<Policies...>::value);
16051645
}
16061646
};
16071647

@@ -1623,7 +1663,8 @@ struct RegisterClassMethod<ReturnType (ThisType, Args...)> {
16231663
reinterpret_cast<GenericFunction>(invoke),
16241664
getContext(callable),
16251665
false,
1626-
isAsync<Policies...>::value);
1666+
isAsync<Policies...>::value,
1667+
isNonnullReturn<Policies...>::value);
16271668
}
16281669
};
16291670

@@ -1752,7 +1793,7 @@ class class_ {
17521793
class_function(
17531794
"implement",
17541795
&wrapped_new<WrapperType*, WrapperType, val, ConstructorArgs...>,
1755-
allow_raw_pointer<ret_val>())
1796+
allow_raw_pointer<ret_val>(), nonnull<ret_val>())
17561797
.class_function(
17571798
"extend",
17581799
&wrapped_extend<WrapperType>)
@@ -1940,7 +1981,8 @@ class class_ {
19401981
getSignature(invoke),
19411982
reinterpret_cast<GenericFunction>(invoke),
19421983
reinterpret_cast<GenericFunction>(classMethod),
1943-
isAsync<Policies...>::value);
1984+
isAsync<Policies...>::value,
1985+
isNonnullReturn<Policies...>::value);
19441986
return *this;
19451987
}
19461988

‎test/code_size/embind_hello_wasm.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
"a.html.gz": 380,
44
"a.js": 9910,
55
"a.js.gz": 4352,
6-
"a.wasm": 7715,
7-
"a.wasm.gz": 3512,
8-
"total": 18177,
9-
"total_gz": 8244
6+
"a.wasm": 7728,
7+
"a.wasm.gz": 3519,
8+
"total": 18190,
9+
"total_gz": 8251
1010
}

‎test/other/embind_tsgen.cpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -90,6 +90,7 @@ int smart_ptr_function(std::shared_ptr<ClassWithSmartPtrConstructor>) {
9090

9191
struct Obj {};
9292
Obj* get_pointer(Obj* ptr) { return ptr; }
93+
Obj* get_nonnull_pointer() { return new Obj(); }
9394

9495
int function_with_callback_param(CallbackType ct) {
9596
ct(val("hello"));
@@ -126,6 +127,18 @@ class DerivedClass : public BaseClass {
126127
int fn2(int x) { return 2; }
127128
};
128129

130+
struct Interface {
131+
virtual void invoke(const std::string& str) = 0;
132+
virtual ~Interface() {}
133+
};
134+
135+
struct InterfaceWrapper : public wrapper<Interface> {
136+
EMSCRIPTEN_WRAPPER(InterfaceWrapper);
137+
void invoke(const std::string& str) {
138+
return call<void>("invoke", str);
139+
}
140+
};
141+
129142
EMSCRIPTEN_BINDINGS(Test) {
130143
class_<Test>("Test")
131144
.function("functionOne", &Test::function_one)
@@ -150,6 +163,7 @@ EMSCRIPTEN_BINDINGS(Test) {
150163
&class_unique_ptr_returning_fn);
151164
class_<Obj>("Obj");
152165
function("getPointer", &get_pointer, allow_raw_pointers());
166+
function("getNonnullPointer", &get_nonnull_pointer, allow_raw_pointers(), nonnull<ret_val>());
153167

154168
constant("an_int", 5);
155169
constant("a_bool", false);
@@ -223,6 +237,11 @@ EMSCRIPTEN_BINDINGS(Test) {
223237

224238
class_<DerivedClass, base<BaseClass>>("DerivedClass")
225239
.function("fn2", &DerivedClass::fn2);
240+
241+
class_<Interface>("Interface")
242+
.function("invoke", &Interface::invoke, pure_virtual())
243+
.allow_subclass<InterfaceWrapper>("InterfaceWrapper")
244+
;
226245
}
227246

228247
int Test::static_property = 42;

0 commit comments

Comments
 (0)