Skip to content
This repository was archived by the owner on Nov 26, 2019. It is now read-only.

Commit 2e891ce

Browse files
implausibleimplausible
committed
Add ignorable callback arguments
This is especially helpful right now, because certain parameters have difficult garbage collection patterns. At the very least, for cases where a garbage collection pattern has not been fully fleshed out, we can ignore certain callback parameters until we can safely handle their memory.
1 parent c187a3d commit 2e891ce

7 files changed

Lines changed: 91 additions & 122 deletions

File tree

‎generate/input/callbacks.json‎

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -57,24 +57,6 @@
5757
"error": -1
5858
}
5959
},
60-
"git_blob_chunk_cb": {
61-
"args": [
62-
{
63-
"name": "entry",
64-
"cType": "const git_config_entry *"
65-
},
66-
{
67-
"name": "payload",
68-
"cType": "void *"
69-
}
70-
],
71-
"return": {
72-
"type": "int",
73-
"noResults": 1,
74-
"success": 0,
75-
"error": -1
76-
}
77-
},
7860
"git_checkout_notify_cb": {
7961
"args": [
8062
{
@@ -326,7 +308,8 @@
326308
"args": [
327309
{
328310
"name": "diff_so_far",
329-
"cType": "const git_diff *"
311+
"cType": "const git_diff *",
312+
"ignore": true
330313
},
331314
{
332315
"name": "delta_to_add",
@@ -347,11 +330,13 @@
347330
"success": 0,
348331
"error": -1
349332
}
350-
},"git_diff_progress_cb": {
333+
},
334+
"git_diff_progress_cb": {
351335
"args": [
352336
{
353337
"name": "diff_so_far",
354-
"cType": "const git_diff *"
338+
"cType": "const git_diff *",
339+
"ignore": true
355340
},
356341
{
357342
"name": "old_path",
@@ -551,7 +536,7 @@
551536
"args": [
552537
{
553538
"name": "out",
554-
"cType": "git_repository **",
539+
"cType": "git_remote **",
555540
"isReturn": true
556541
},
557542
{

‎generate/scripts/generateNativeCode.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,8 @@ module.exports = function generateNativeCode() {
5252
argsInfo: require("../templates/filters/args_info"),
5353
arrayTypeToPlainType: require("../templates/filters/array_type_to_plain_type"),
5454
asElementPointer: require("../templates/filters/as_element_pointer"),
55+
callbackArgsInfo: require("../templates/filters/callback_args_info"),
56+
callbackArgsCount: require("../templates/filters/callback_args_count"),
5557
cppToV8: require("../templates/filters/cpp_to_v8"),
5658
defaultValue: require("../templates/filters/default_value"),
5759
fieldsInfo: require("../templates/filters/fields_info"),

‎generate/templates/filters/callback_args_count.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
module.exports = function(args) {
2+
if (!args) {
3+
return 0;
4+
}
5+
6+
return args.reduce(
7+
function(count, arg) {
8+
var shouldCount = !arg.isReturn &&
9+
!arg.isSelf &&
10+
arg.name !== "payload" &&
11+
arg.name !== "self" &&
12+
!arg.ignore;
13+
14+
return shouldCount ? count + 1 : count;
15+
},
16+
0
17+
);
18+
};

‎generate/templates/filters/callback_args_info.js‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
module.exports = function(args) {
2+
var result = args.reduce(
3+
function(argList, arg) {
4+
var useArg = !arg.isReturn &&
5+
!arg.isSelf &&
6+
arg.name !== "payload" &&
7+
arg.name !== "self" &&
8+
!arg.ignore;
9+
10+
if (!useArg) {
11+
return argList;
12+
}
13+
14+
arg.firstArg = argList.length === 0;
15+
argList.push(arg);
16+
17+
return argList;
18+
},
19+
[]
20+
);
21+
22+
if (result.length) {
23+
result[result.length - 1].lastArg = true;
24+
}
25+
26+
return result;
27+
};

‎generate/templates/filters/js_args_count.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ module.exports = function(args) {
55
if (!args) {
66
return 0;
77
}
8-
8+
99
for(cArg = 0, jsArg = 0; cArg < args.length; cArg++) {
1010
var arg = args[cArg];
1111

12-
if (!arg.isReturn && !arg.isSelf && !arg.isPayload) {
12+
if (!arg.isReturn && !arg.isSelf) {
1313
jsArg++;
1414
}
1515
}

‎generate/templates/partials/callback_helpers.cc‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,32 +30,27 @@ void {{ cppClassName }}::{{ cppFunctionName }}_{{ cbFunction.name }}_async(void
3030
{% endif %}
3131
{% endeach %}
3232

33-
v8::Local<Value> argv[{{ cbFunction.args|jsArgsCount }}] = {
34-
{% each cbFunction.args|argsInfo as arg %}
35-
{% if arg | isPayload %}
36-
{%-- payload is always the last arg --%}
37-
// payload is null because we can use closure scope in javascript
38-
Nan::Undefined()
39-
{% elsif arg.isJsArg %}
40-
{% if arg.isEnum %}
41-
Nan::New((int)baton->{{ arg.name }}),
42-
{% elsif arg.isLibgitType %}
43-
{{ arg.cppClassName }}::New(baton->{{ arg.name }}, false),
44-
{% elsif arg.cType == "size_t" %}
45-
// HACK: NAN should really have an overload for Nan::New to support size_t
46-
Nan::New((unsigned int)baton->{{ arg.name }}),
47-
{% elsif arg.cppClassName == 'String' %}
48-
Nan::New(baton->{{ arg.name }}).ToLocalChecked(),
49-
{% else %}
50-
Nan::New(baton->{{ arg.name }}),
51-
{% endif %}
33+
v8::Local<Value> argv[{{ cbFunction.args|callbackArgsCount }}] = {
34+
{% each cbFunction.args|callbackArgsInfo as arg %}
35+
{% if not arg.firstArg %}, {% endif %}
36+
{% if arg.isEnum %}
37+
Nan::New((int)baton->{{ arg.name }})
38+
{% elsif arg.isLibgitType %}
39+
{{ arg.cppClassName }}::New(baton->{{ arg.name }}, false)
40+
{% elsif arg.cType == "size_t" %}
41+
// HACK: NAN should really have an overload for Nan::New to support size_t
42+
Nan::New((unsigned int)baton->{{ arg.name }})
43+
{% elsif arg.cppClassName == 'String' %}
44+
Nan::New(baton->{{ arg.name }}).ToLocalChecked()
45+
{% else %}
46+
Nan::New(baton->{{ arg.name }})
5247
{% endif %}
5348
{% endeach %}
5449
};
5550

5651
Nan::TryCatch tryCatch;
5752
// TODO This should take an async_resource, but we will need to figure out how to pipe the correct context into this
58-
Nan::MaybeLocal<v8::Value> maybeResult = Nan::Call(*callback, {{ cbFunction.args|jsArgsCount }}, argv);
53+
Nan::MaybeLocal<v8::Value> maybeResult = Nan::Call(*callback, {{ cbFunction.args|callbackArgsCount }}, argv);
5954
v8::Local<v8::Value> result;
6055
if (!maybeResult.IsEmpty()) {
6156
result = maybeResult.ToLocalChecked();

‎generate/templates/partials/field_accessors.cc‎

Lines changed: 20 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -179,92 +179,34 @@
179179
return;
180180
}
181181

182-
{% each field.args|argsInfo as arg %}
183-
{% if arg.name == "payload" %}
184-
{%-- Do nothing --%}
185-
{% elsif arg.isJsArg %}
186-
{% if arg.cType == "const char *" %}
187-
if (baton->{{ arg.name }} == NULL) {
188-
baton->{{ arg.name }} = "";
189-
}
190-
{% elsif arg.cppClassName == "String" %}
191-
v8::Local<v8::Value> src;
192-
if (baton->{{ arg.name }} == NULL) {
193-
src = Nan::Null();
194-
}
195-
else {
196-
src = Nan::New<String>(*baton->{{ arg.name }}).ToLocalChecked();
197-
}
198-
{% endif %}
199-
{% endif %}
200-
{% endeach %}
201-
202-
{% if field.isSelfReferential %}
203-
{% if field.args|jsArgsCount|subtract 2| setUnsigned == 0 %}
204-
v8::Local<Value> *argv = NULL;
205-
{% else %}
206-
v8::Local<Value> argv[{{ field.args|jsArgsCount|subtract 2| setUnsigned }}] = {
207-
{% endif %}
182+
{% if field.args|callbackArgsCount == 0 %}
183+
v8::Local<Value> *argv = NULL;
208184
{% else %}
209-
v8::Local<Value> argv[{{ field.args|jsArgsCount }}] = {
210-
{% endif %}
211-
{% each field.args|argsInfo as arg %}
212-
{% if field.isSelfReferential %}
213-
{% if not arg.firstArg %}
214-
{% if field.args|jsArgsCount|subtract 1|or 0 %}
215-
{% if arg.cppClassName == "String" %}
216-
{%-- src is always the last arg --%}
217-
src
218-
{% elsif arg.isJsArg %}
219-
{% if arg.isEnum %}
220-
Nan::New((int)baton->{{ arg.name }}),
221-
{% elsif arg.isLibgitType %}
222-
{{ arg.cppClassName }}::New(baton->{{ arg.name }}, false),
223-
{% elsif arg.cType == "size_t" %}
224-
Nan::New((unsigned int)baton->{{ arg.name }}),
225-
{% elsif arg.name == "payload" %}
226-
{%-- skip, filters should not have a payload --%}
227-
{% else %}
228-
Nan::New(baton->{{ arg.name }}),
229-
{% endif %}
230-
{% endif %}
231-
{% endif %}
232-
{% endif %}
233-
{% else %}
234-
{% if arg.name == "payload" %}
235-
{%-- payload is always the last arg --%}
236-
Nan::New(instance->{{ fields|payloadFor field.name }})
237-
{% elsif arg.isJsArg %}
238-
{% if arg.isEnum %}
239-
Nan::New((int)baton->{{ arg.name }}),
240-
{% elsif arg.isLibgitType %}
241-
{{ arg.cppClassName }}::New(baton->{{ arg.name }}, false),
242-
{% elsif arg.cType == "size_t" %}
243-
// HACK: NAN should really have an overload for Nan::New to support size_t
244-
Nan::New((unsigned int)baton->{{ arg.name }}),
245-
{% elsif arg.cppClassName == "String" %}
246-
Nan::New(baton->{{ arg.name }}).ToLocalChecked(),
247-
{% else %}
248-
Nan::New(baton->{{ arg.name }}),
249-
{% endif %}
185+
v8::Local<Value> argv[{{ field.args|callbackArgsCount }}] = {
186+
{% each field.args|callbackArgsInfo as arg %}
187+
{% if not arg.firstArg %},{% endif %}
188+
{% if arg.isEnum %}
189+
Nan::New((int)baton->{{ arg.name }})
190+
{% elsif arg.isLibgitType %}
191+
{{ arg.cppClassName }}::New(baton->{{ arg.name }}, false)
192+
{% elsif arg.cType == "size_t" %}
193+
// HACK: NAN should really have an overload for Nan::New to support size_t
194+
Nan::New((unsigned int)baton->{{ arg.name }})
195+
{% elsif arg.cppClassName == "String" %}
196+
baton->{{ arg.name }} == NULL
197+
? Nan::EmptyString()
198+
: Nan::New({%if arg.cType | isDoublePointer %}*{% endif %}baton->{{ arg.name }}).ToLocalChecked()
199+
{% else %}
200+
Nan::New(baton->{{ arg.name }})
250201
{% endif %}
251-
{% endif %}
252-
{% endeach %}
253-
{% if not field.isSelfReferential %}
254-
};
255-
{% elsif field.args|jsArgsCount|subtract 2| setUnsigned > 0 %}
202+
{% endeach %}
256203
};
257204
{% endif %}
258205

259206
Nan::TryCatch tryCatch;
260207

261208
// TODO This should take an async_resource, but we will need to figure out how to pipe the correct context into this
262-
{% if field.isSelfReferential %}
263-
Nan::MaybeLocal<v8::Value> maybeResult = Nan::Call(*(instance->{{ field.name }}.GetCallback()), {{ field.args|jsArgsCount|subtract 2| setUnsigned }}, argv);
264-
{% else %}
265-
Nan::MaybeLocal<v8::Value> maybeResult = Nan::Call(*(instance->{{ field.name }}.GetCallback()), {{ field.args|jsArgsCount }}, argv);
266-
{% endif %}
267-
209+
Nan::MaybeLocal<v8::Value> maybeResult = Nan::Call(*(instance->{{ field.name }}.GetCallback()), {{ field.args|callbackArgsCount }}, argv);
268210
v8::Local<v8::Value> result;
269211
if (!maybeResult.IsEmpty()) {
270212
result = maybeResult.ToLocalChecked();

0 commit comments

Comments
 (0)