Skip to content

Commit e651147

Browse files
brendandahlbrendandahl
committed
[embind] Support annotating return values as nonnull.
Embind's subclass `implement` methods were generated as returning `Class | null` after the changes to pointer types in #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 d0d9966 commit e651147

File tree

8 files changed

+153
-24
lines changed

8 files changed

+153
-24
lines changed

‎src/embind/embind.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -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)}`);
@@ -463,7 +463,7 @@ var LibraryEmbind = {
463463
registerType(id, new IntegerType(id));
464464
},
465465
$createFunctionDefinition__deps: ['$FunctionDefinition', '$heap32VectorToArray', '$readLatin1String', '$Argument', '$whenDependentTypesAreResolved', '$getFunctionName', '$getFunctionArgsName', '$PointerDefinition', '$ClassDefinition'],
466-
$createFunctionDefinition: (name, argCount, rawArgTypesAddr, functionIndex, hasThis, isConstructor, isAsync, cb) => {
466+
$createFunctionDefinition: (name, argCount, rawArgTypesAddr, functionIndex, hasThis, isNonnullReturn, isAsync, cb) => {
467467
const argTypes = heap32VectorToArray(argCount, rawArgTypesAddr);
468468
name = typeof name === 'string' ? name : readLatin1String(name);
469469

@@ -493,7 +493,7 @@ var LibraryEmbind = {
493493
args.push(new Argument(`_${i - argStart}`, argTypes[i]));
494494
}
495495
}
496-
const funcDef = new FunctionDefinition(name, returnType, args, functionIndex, thisType, isConstructor, isAsync);
496+
const funcDef = new FunctionDefinition(name, returnType, args, functionIndex, thisType, isNonnullReturn, isAsync);
497497
cb(funcDef);
498498
return [];
499499
});
@@ -544,8 +544,8 @@ var LibraryEmbind = {
544544
// TODO
545545
},
546546
_embind_register_function__deps: ['$moduleDefinitions', '$createFunctionDefinition'],
547-
_embind_register_function: (name, argCount, rawArgTypesAddr, signature, rawInvoker, fn, isAsync) => {
548-
createFunctionDefinition(name, argCount, rawArgTypesAddr, fn, false, false, isAsync, (funcDef) => {
547+
_embind_register_function: (name, argCount, rawArgTypesAddr, signature, rawInvoker, fn, isAsync, isNonnullReturn) => {
548+
createFunctionDefinition(name, argCount, rawArgTypesAddr, fn, false, isNonnullReturn, isAsync, (funcDef) => {
549549
moduleDefinitions.push(funcDef);
550550
});
551551
},
@@ -605,8 +605,9 @@ var LibraryEmbind = {
605605
rawInvoker,
606606
context,
607607
isPureVirtual,
608-
isAsync) {
609-
createFunctionDefinition(methodName, argCount, rawArgTypesAddr, context, true, false, isAsync, (funcDef) => {
608+
isAsync,
609+
isNonnullReturn) {
610+
createFunctionDefinition(methodName, argCount, rawArgTypesAddr, context, true, isNonnullReturn, isAsync, (funcDef) => {
610611
const classDef = funcDef.thisType;
611612
classDef.methods.push(funcDef);
612613
});
@@ -644,10 +645,11 @@ var LibraryEmbind = {
644645
invokerSignature,
645646
rawInvoker,
646647
fn,
647-
isAsync) {
648+
isAsync,
649+
isNonnullReturn) {
648650
whenDependentTypesAreResolved([], [rawClassType], function(classType) {
649651
classType = classType[0];
650-
createFunctionDefinition(methodName, argCount, rawArgTypesAddr, fn, false, false, isAsync, (funcDef) => {
652+
createFunctionDefinition(methodName, argCount, rawArgTypesAddr, fn, false, isNonnullReturn, isAsync, (funcDef) => {
651653
classType.staticMethods.push(funcDef);
652654
});
653655
return [];

‎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/other/embind_tsgen.cpp‎

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

9292
struct Obj {};
9393
Obj* get_pointer(Obj* ptr) { return ptr; }
94+
Obj* get_nonnull_pointer() { return new Obj(); }
9495

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

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

155169
constant("an_int", 5);
156170
constant("a_bool", false);
@@ -225,6 +239,11 @@ EMSCRIPTEN_BINDINGS(Test) {
225239

226240
class_<DerivedClass, base<BaseClass>>("DerivedClass")
227241
.function("fn2", &DerivedClass::fn2);
242+
243+
class_<Interface>("Interface")
244+
.function("invoke", &Interface::invoke, pure_virtual())
245+
.allow_subclass<InterfaceWrapper>("InterfaceWrapper")
246+
;
228247
}
229248

230249
int Test::static_property = 42;

‎test/other/embind_tsgen.d.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,16 @@ export interface DerivedClass extends BaseClass {
9595
delete(): void;
9696
}
9797

98+
export interface Interface {
99+
invoke(_0: EmbindString): void;
100+
delete(): void;
101+
}
102+
103+
export interface InterfaceWrapper extends Interface {
104+
notifyOnDestruction(): void;
105+
delete(): void;
106+
}
107+
98108
export type ValArr = [ number, number, number ];
99109

100110
export type ValObj = {
@@ -117,6 +127,7 @@ interface EmbindModule {
117127
class_unique_ptr_returning_fn(): Test;
118128
Obj: {};
119129
getPointer(_0: Obj | null): Obj | null;
130+
getNonnullPointer(): Obj;
120131
a_class_instance: Test;
121132
an_enum: Bar;
122133
Bar: {valueOne: BarValue<0>, valueTwo: BarValue<1>, valueThree: BarValue<2>};
@@ -141,6 +152,11 @@ interface EmbindModule {
141152
};
142153
BaseClass: {};
143154
DerivedClass: {};
155+
Interface: {
156+
implement(_0: any): InterfaceWrapper;
157+
extend(_0: EmbindString, _1: any): any;
158+
};
159+
InterfaceWrapper: {};
144160
a_bool: boolean;
145161
an_int: number;
146162
optional_test(_0?: Foo): number | undefined;

‎test/other/embind_tsgen_ignore_1.d.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,16 @@ export interface DerivedClass extends BaseClass {
104104
delete(): void;
105105
}
106106

107+
export interface Interface {
108+
invoke(_0: EmbindString): void;
109+
delete(): void;
110+
}
111+
112+
export interface InterfaceWrapper extends Interface {
113+
notifyOnDestruction(): void;
114+
delete(): void;
115+
}
116+
107117
export type ValArr = [ number, number, number ];
108118

109119
export type ValObj = {
@@ -126,6 +136,7 @@ interface EmbindModule {
126136
class_unique_ptr_returning_fn(): Test;
127137
Obj: {};
128138
getPointer(_0: Obj | null): Obj | null;
139+
getNonnullPointer(): Obj;
129140
a_class_instance: Test;
130141
an_enum: Bar;
131142
Bar: {valueOne: BarValue<0>, valueTwo: BarValue<1>, valueThree: BarValue<2>};
@@ -150,6 +161,11 @@ interface EmbindModule {
150161
};
151162
BaseClass: {};
152163
DerivedClass: {};
164+
Interface: {
165+
implement(_0: any): InterfaceWrapper;
166+
extend(_0: EmbindString, _1: any): any;
167+
};
168+
InterfaceWrapper: {};
153169
a_bool: boolean;
154170
an_int: number;
155171
optional_test(_0?: Foo): number | undefined;

0 commit comments

Comments
 (0)