Inline functions with apparent name collisions

[stasm]

Bug report or Feature request?

Feature request. When the callee's parameters are named the same as the arguments and no reassignment happens in its body, would it be possible to inline the callee into the caller?

I think this is related to #639 (which mentions Terser 5) and #656. There's also #661 which mentions variable renaming; it says it was merged into master but I guess it's on feature/terser-5 now?

Version (complete output of terser -V or specific git commit)

terser 4.8.0

Complete CLI command or minify() options used

terser -c toplevel input.js

terser input

function entry() {
    let arr = [];
    walk(arr);
}

// This function is inlined.
function walk(arr) {
    for (let elem of arr) {
        if (!elem.seen) {
            mark(elem);
        }
    }
}

// This function isn't.
function mark(elem) {
    elem.seen = true;
}

entry();

terser output or error

function mark(elem){elem.seen=!0}!function(arr){for(let elem of arr)elem.seen||mark(elem)}([]);

Expected result

!function(arr){for(let elem of arr)elem.seen||(elem.seen=!0)}([]);

[fabiosantoscode]

Hey there!

Some things are more complicated to inline, so they aren't inlined. For example, when you're inlining something like x => something(x) into a place where x already exists, it's a bit more complicated.

This is almost a duplicate of #656, except that in that particular issue, the fix is to unconditionally obey the /*#__INLINE__*/ instruction.

I have to close this, because while it's completely possible to fix, it would be a further maintenance burden.

Thanks for reporting!

[stasm]

This is almost a duplicate of #656, except that in that particular issue, the fix is to unconditionally obey the /#INLINE/ instruction.

Do you mean the fix to #656 or #744, i.e. this issue? Do you plan to add unconditional inlining in #656? The outcome of the discussion there isn't clear to me. If there was a way to force inlining unconditionally, as a developer I could opt into more aggressive inlining at callsites which I know are safe (like the snippet in #744 (comment)).

I understand that proper inlining can be tricky and I also understand the decision to avoid maintenance burden. Just curious: outside inlining the body of the callee verbatim and renaming parameters to to avoid conflicts, have you considered renaming parameters to the names of the arguments used by the caller?

[fabiosantoscode]

I just mean the code that causes the example in #656 to be suboptimal is the same code at question in this issue :)

Yes #656 will result in unconditional inlining, although I don't really have a timeline for it. Depends on what bugs and new ES features pop up and how much time I have to deal with it.

have you considered renaming parameters to the names of the arguments used by the caller?

That was my best shot at fixing the problem. It sounded great to me, but in the end it turned out to be really tricky in practice, due to limitations of how Terser itself is designed (the JS syntax tree is mutated during runtime, and sometimes scope and reference graph information is lost) so I never pushed it.

I am slowly working to solve these mutability-related problems with a more hollistic approach to reference and scope management in the face of transforms, and that's why Terser 5 will not expose the internal AST.

[stasm]

Thank you for providing more context on this, it's interesting to learn about the design of Terser! I'll keep fingers crossed for the future improvements!