-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Open
Labels
P4featFeature RequestFeature Requestgood first issueRelatively easy, but low priority bugsRelatively easy, but low priority bugshelp wanted
Description
Consider the code:
/** @const {!Array<number>} */
const a = [1];
/** @const {!Array<number>} */
const b = Array(10);
b[0] = 2;
/** @const {!Array<number>} */
const c = [];
c.push(3);
/** @const {!Array<number>} */
const d = Array(10);
d.push(4);
When compiled with
yarn google-closure-compiler -O ADVANCED --js a.js --rewrite_polyfills=false
we get
Array(10)[0]=2;Array(10).push(4);
Arrays created with the Array(len)
constructor are not eliminated.
google-closure-compiler version: v20230103
Metadata
Metadata
Assignees
Labels
P4featFeature RequestFeature Requestgood first issueRelatively easy, but low priority bugsRelatively easy, but low priority bugshelp wanted
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
brad4d commentedon Jan 23, 2023
The peephole optimizations remove the cases of an array created with an array literal.
Presumably we could update whichever one does that so it also removes arrays created with the
Array()
constructor.However, I don't think we're likely to prioritize fixing this.
It's just not likely to be worth the effort.
I'll mark this as "help wanted", since I think it would be a good small contribution that could be made from outside our team.
KimlikDAO-bot commentedon Jan 23, 2023
Thank you! We'll look into fixing this
jobedrony commentedon Aug 6, 2023
I want to work in this issue
PTheocharis commentedon Mar 15, 2024
Is it possible I work on this issue?
KimlikDAO-bot commentedon May 29, 2024
@brad4d Could you point to some relevant files in the code?
brad4d commentedon Jun 4, 2024
I would start by adding a test case here:
closure-compiler/test/com/google/javascript/jscomp/PeepholeRemoveDeadCodeTest.java
Line 1476 in d77e3ad
Then investigate how to make
PeepholeRemoveDeadCode
pass this test.KimlikDAO-bot commentedon Mar 2, 2025
@brad4d Actually that test passes. The difference between
[]
andArray()
constructor is only visible if we do something like[undefined].push(1)
vsArray(1).push(1)
.brad4d commentedon Mar 7, 2025
In that case, you should start with a test case containing those two cases and use the debugger to investigate what it is that prevents the
Array(1).push(1);
from being removed but allows[undefined].push(1);
to be removed.