Skip to content

Commit 65cfbfa

Browse files
authored
Simplify modifyFunction helper function. NFC (#19551)
The name of the function is not important at any of the call sites since we only use this helper to modify JS library functions, and JS library functions get their name set explicitly at the top of `processLibraryFunction`. Also, rename the function just in case there are external caller of this function, this will make it clear they need to update their code. This change was motivated by #19539 where it helps to avoid worrying about functions names.
1 parent 7972c8e commit 65cfbfa

7 files changed

+30
-28
lines changed

ChangeLog.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,10 @@ See docs/process.md for more on how version tagging works.
2828
will be included that fail at runtime. This matches the behaviour of other
2929
libc functions that we don't implement. For those that prefer to get a linker
3030
error we have the `-sALLOW_UNIMPLEMENTED_SYSCALLS` settings. (#19527)
31+
- The `modifyFunction` helper in `parseTools.js` was renamed to
32+
`modifyJSFunction` and its callback function no longer takes the name of the
33+
function being modified. The name is not relevant for JS library functions
34+
and can be safely ignored.
3135

3236
3.1.41 - 06/06/23
3337
-----------------

src/jsifier.js

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ function runJSify() {
112112
// Automatically convert any incoming pointer arguments from BigInt
113113
// to double (this limits the range to int53).
114114
// And convert the return value if the function returns a pointer.
115-
return modifyFunction(snippet, (name, args, body) => {
115+
return modifyJSFunction(snippet, (args, body) => {
116116
let argLines = args.split('\n');
117117
argLines = argLines.map((line) => line.split('//')[0]);
118118
const argNames = argLines.join(' ').split(',').map((name) => name.trim());
@@ -138,7 +138,7 @@ function runJSify() {
138138
// body in an inner function.
139139
newArgs = newArgs.join(',');
140140
return `\
141-
function ${name}(${args}) {
141+
function(${args}) {
142142
var ret = ((${args}) => { ${body} })(${newArgs});
143143
return BigInt(ret);
144144
}`;
@@ -147,7 +147,7 @@ function ${name}(${args}) {
147147
// Otherwise no inner function is needed and we covert the arguments
148148
// before executing the function body.
149149
return `\
150-
function ${name}(${args}) {
150+
function(${args}) {
151151
${argConvertions}
152152
${body};
153153
}`;
@@ -162,17 +162,14 @@ ${argConvertions}
162162
// uniform.
163163
snippet = snippet.toString().replace(/\r\n/gm, '\n');
164164

165-
// name the function; overwrite if it's already named
166-
snippet = snippet.replace(/function(?:\s+([^(]+))?\s*\(/, 'function ' + mangled + '(');
167-
168165
if (isStub) {
169166
return snippet;
170167
}
171168

172169
// apply LIBRARY_DEBUG if relevant
173170
if (LIBRARY_DEBUG && !isJsOnlySymbol(symbol)) {
174-
snippet = modifyFunction(snippet, (name, args, body) => `\
175-
function ${name}(${args}) {
171+
snippet = modifyJSFunction(snippet, (args, body) => `\
172+
function(${args}) {
176173
var ret = (function() { if (runtimeDebug) err("[library call:${mangled}: " + Array.prototype.slice.call(arguments).map(prettyPrint) + "]");
177174
${body}
178175
}).apply(this, arguments);
@@ -189,18 +186,18 @@ function ${name}(${args}) {
189186
}
190187
const sync = proxyingMode === 'sync';
191188
if (PTHREADS) {
192-
snippet = modifyFunction(snippet, (name, args, body) => `
193-
function ${name}(${args}) {
189+
snippet = modifyJSFunction(snippet, (args, body) => `
190+
function(${args}) {
194191
if (ENVIRONMENT_IS_PTHREAD)
195192
return proxyToMainThread(${proxiedFunctionTable.length}, ${+sync}${args ? ', ' : ''}${args});
196193
${body}
197194
}\n`);
198195
} else if (WASM_WORKERS && ASSERTIONS) {
199196
// In ASSERTIONS builds add runtime checks that proxied functions are not attempted to be called in Wasm Workers
200197
// (since there is no automatic proxying architecture available)
201-
snippet = modifyFunction(snippet, (name, args, body) => `
202-
function ${name}(${args}) {
203-
assert(!ENVIRONMENT_IS_WASM_WORKER, "Attempted to call proxied function '${name}' in a Wasm Worker, but in Wasm Worker enabled builds, proxied function architecture is not available!");
198+
snippet = modifyJSFunction(snippet, (args, body) => `
199+
function(${args}) {
200+
assert(!ENVIRONMENT_IS_WASM_WORKER, "Attempted to call proxied function '${mangled}' in a Wasm Worker, but in Wasm Worker enabled builds, proxied function architecture is not available!");
204201
${body}
205202
}\n`);
206203
}
@@ -437,8 +434,8 @@ function ${name}(${args}) {
437434
if (isFunction) {
438435
// Emit the body of a JS library function.
439436
if ((USE_ASAN || USE_LSAN || UBSAN_RUNTIME) && LibraryManager.library[symbol + '__noleakcheck']) {
440-
contentText = modifyFunction(snippet, (name, args, body) => `
441-
function ${name}(${args}) {
437+
contentText = modifyJSFunction(snippet, (args, body) => `
438+
function(${args}) {
442439
return withBuiltinMalloc(function() {
443440
${body}
444441
});
@@ -447,6 +444,10 @@ function ${name}(${args}) {
447444
} else {
448445
contentText = snippet; // Regular JS function that will be executed in the context of the calling thread.
449446
}
447+
// Give the function the correct (mangled) name. Overwrite it if it's
448+
// already named. This must happen after the last call to
449+
// modifyJSFunction which could have changed or removed the name.
450+
contentText = contentText.replace(/function(?:\s+([^(]+))?\s*\(/, `function ${mangled}(`);
450451
} else if (typeof snippet == 'string' && snippet.startsWith(';')) {
451452
// In JS libraries
452453
// foo: ';[code here verbatim]'

src/library.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3799,8 +3799,8 @@ function wrapSyscallFunction(x, library, isWasi) {
37993799
// has disabled the filesystem or we have proven some other way that this will
38003800
// not be called in practice, and do not need that code.
38013801
if (!SYSCALLS_REQUIRE_FILESYSTEM && t.includes('FS.')) {
3802-
t = modifyFunction(t, function(name, args, body) {
3803-
return 'function ' + name + '(' + args + ') {\n' +
3802+
t = modifyJSFunction(t, function(args, body) {
3803+
return `function(${args}) {\n` +
38043804
(ASSERTIONS ? "abort('it should not be possible to operate on streams when !SYSCALLS_REQUIRE_FILESYSTEM');\n" : '') +
38053805
'}';
38063806
});
@@ -3871,8 +3871,8 @@ function wrapSyscallFunction(x, library, isWasi) {
38713871
post = handler + post;
38723872

38733873
if (pre || post) {
3874-
t = modifyFunction(t, function(name, args, body) {
3875-
return `function ${name}(${args}) {\n${pre}${body}${post}}\n`;
3874+
t = modifyJSFunction(t, function(args, body) {
3875+
return `function(${args}) {\n${pre}${body}${post}}\n`;
38763876
});
38773877
}
38783878

src/parseTools.js

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -684,26 +684,23 @@ function makeRetainedCompilerSettings() {
684684
const WASM_PAGE_SIZE = 65536;
685685

686686
// Receives a function as text, and a function that constructs a modified
687-
// function, to which we pass the parsed-out name, arguments, body, and possible
687+
// function, to which we pass the parsed-out arguments, body, and possible
688688
// "async" prefix of the input function. Returns the output of that function.
689-
function modifyFunction(text, func) {
689+
function modifyJSFunction(text, func) {
690690
// Match a function with a name.
691691
let match = text.match(/^\s*(async\s+)?function\s+([^(]*)?\s*\(([^)]*)\)/);
692692
let async_;
693-
let name;
694693
let args;
695694
let rest;
696695
if (match) {
697696
async_ = match[1] || '';
698-
name = match[2];
699697
args = match[3];
700698
rest = text.substr(match[0].length);
701699
} else {
702700
// Match a function without a name (we could probably use a single regex
703701
// for both, but it would be more complex).
704702
match = text.match(/^\s*(async\s+)?function\(([^)]*)\)/);
705703
assert(match, 'could not match function ' + text + '.');
706-
name = '';
707704
async_ = match[1] || '';
708705
args = match[2];
709706
rest = text.substr(match[0].length);
@@ -712,7 +709,7 @@ function modifyFunction(text, func) {
712709
assert(bodyStart >= 0);
713710
const bodyEnd = rest.lastIndexOf('}');
714711
assert(bodyEnd > 0);
715-
return func(name, args, rest.substring(bodyStart + 1, bodyEnd), async_);
712+
return func(args, rest.substring(bodyStart + 1, bodyEnd), async_);
716713
}
717714

718715
function runIfMainThread(text) {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
59352
1+
59349
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
33004
1+
33001
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
58318
1+
58315

0 commit comments

Comments
 (0)