Skip to content

Commit 53d8fd1

Browse files
authored
Improve comment preservation in acorn-optimizer. NFC (#22136)
Prior to this change we would drop any comment that was more than 20 characters from JS symbol, based on the end of the commend and that start of the symbol. This mean that all but the last single-line comments in a sequence would be dropped. With this change we map all comments to the start of the next symbol. This means that we can have many comments in row that all map the same location and we decide to either keep all of them or discard them.
1 parent 5028d4f commit 53d8fd1

File tree

3 files changed

+62
-33
lines changed

3 files changed

+62
-33
lines changed

test/optimizer/test-unsignPointers-output.js

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,14 @@
11
/**
2-
* This is a multi-line comment
2+
* Multi-line comment.
3+
*/ /**
4+
* Another multi-line comment.
5+
*/ /**
6+
* A third multi-line comment.
37
*/ HEAP32[x >>> 2];
48

5-
// This is a single-line comment
9+
// Single line comment
10+
// Another single line comment
11+
// A third single line comment
612
HEAP8[x >>> 0];
713

814
HEAP8.length;

test/optimizer/test-unsignPointers.js

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
/**
2-
* This is a multi-line comment
2+
* Multi-line comment.
3+
*/
4+
/**
5+
* Another multi-line comment.
6+
*/
7+
/**
8+
* A third multi-line comment.
39
*/
410
HEAP32[x >> 2];
511

6-
// This is a single-line comment
12+
// Single line comment
13+
// Another single line comment
14+
// A third single line comment
715
HEAP8[x];
816

917
HEAP8.length;

tools/acorn-optimizer.mjs

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1932,7 +1932,7 @@ function minifyGlobals(ast) {
19321932

19331933
// Utilities
19341934

1935-
function reattachComments(ast, comments) {
1935+
function reattachComments(ast, commentsMap) {
19361936
const symbols = [];
19371937

19381938
// Collect all code symbols
@@ -1949,33 +1949,37 @@ function reattachComments(ast, comments) {
19491949

19501950
// Walk through all comments in ascending line number, and match each
19511951
// comment to the appropriate code block.
1952-
for (let i = 0, j = 0; i < comments.length; ++i) {
1953-
while (j < symbols.length && symbols[j].start.pos < comments[i].end) {
1952+
let j = 0;
1953+
for (const [pos, comments] of Object.entries(commentsMap)) {
1954+
while (j < symbols.length && symbols[j].start.pos < pos) {
19541955
++j;
19551956
}
19561957
if (j >= symbols.length) {
1957-
trace(`dropping comment: no symbol comes after it (${comments[i].value.slice(0, 30)})`);
1958+
trace('dropping comments: no symbol comes after them');
19581959
break;
19591960
}
1960-
if (symbols[j].start.pos - comments[i].end > 20) {
1961+
if (symbols[j].start.pos - pos > 20) {
19611962
// This comment is too far away to refer to the given symbol. Drop
19621963
// the comment altogether.
1963-
trace(`dropping comment: too far from any symbol (${comments[i].value.slice(0, 30)})`);
1964+
trace('dropping comments: too far from any symbol');
19641965
continue;
19651966
}
19661967
symbols[j].start.comments_before ??= [];
1967-
symbols[j].start.comments_before.push(
1968-
new terser.AST_Token(
1969-
comments[i].type == 'Line' ? 'comment1' : 'comment2',
1970-
comments[i].value,
1971-
undefined,
1972-
undefined,
1973-
false,
1974-
undefined,
1975-
undefined,
1976-
'0',
1977-
),
1978-
);
1968+
for (const comment of comments) {
1969+
trace('reattaching comment');
1970+
symbols[j].start.comments_before.push(
1971+
new terser.AST_Token(
1972+
comment.type == 'Line' ? 'comment1' : 'comment2',
1973+
comment.value,
1974+
undefined,
1975+
undefined,
1976+
false,
1977+
undefined,
1978+
undefined,
1979+
'0',
1980+
),
1981+
);
1982+
}
19791983
}
19801984
}
19811985

@@ -2029,19 +2033,30 @@ let extraInfo = null;
20292033
if (extraInfoStart > 0) {
20302034
extraInfo = JSON.parse(input.substr(extraInfoStart + 14));
20312035
}
2032-
// Collect all JS code comments to this array so that we can retain them in the outputted code
2033-
// if --closureFriendly was requested.
2034-
const sourceComments = [];
2036+
// Collect all JS code comments to this map so that we can retain them in the
2037+
// outputted code if --closureFriendly was requested.
2038+
const sourceComments = {};
2039+
const params = {
2040+
// Keep in sync with --language_in that we pass to closure in building.py
2041+
ecmaVersion: 2021,
2042+
sourceType: exportES6 ? 'module' : 'script',
2043+
allowAwaitOutsideFunction: true,
2044+
};
2045+
if (closureFriendly) {
2046+
const currentComments = [];
2047+
Object.assign(params, {
2048+
preserveParens: true,
2049+
onToken: (token) => {
2050+
// Associate comments with the start position of the next token.
2051+
sourceComments[token.start] = currentComments.slice();
2052+
currentComments.length = 0;
2053+
},
2054+
onComment: currentComments,
2055+
});
2056+
}
20352057
let ast;
20362058
try {
2037-
ast = acorn.parse(input, {
2038-
// Keep in sync with --language_in that we pass to closure in building.py
2039-
ecmaVersion: 2021,
2040-
preserveParens: closureFriendly,
2041-
onComment: closureFriendly ? sourceComments : undefined,
2042-
sourceType: exportES6 ? 'module' : 'script',
2043-
allowAwaitOutsideFunction: true,
2044-
});
2059+
ast = acorn.parse(input, params);
20452060
} catch (err) {
20462061
err.message += (() => {
20472062
let errorMessage = '\n' + input.split(acorn.lineBreak)[err.loc.line - 1] + '\n';

0 commit comments

Comments
 (0)