side_effects=false,pure_getters causes runtime error

[lydell]

Uglify version (uglifyjs -V)
uglify-js 3.17.4

JavaScript input

var hasFocusStyle = function (attr) {
  if (attr.$ === 4 && attr.b.$ === 11 && !attr.b.a) {
    var _v1 = attr.b;
    var _v2 = _v1.a;
    return true;
  } else {
    return false;
  }
};

hasFocusStyle({ $: 7, a: { $: 2, a: 1 } }); // should be `false`

The uglifyjs CLI command executed or minify() options used.

uglifyjs repro.js -c 'side_effects=false,pure_getters'

JavaScript output or error produced.

var hasFocusStyle=function(attr){return attr.b.a,4===attr.$&&11===attr.b.$&&!attr.b.a&&!0};hasFocusStyle({$:7,a:{$:2,a:1}});

TypeError: Cannot read properties of undefined (reading 'a')

Terminal session, showing before and after:

❯ cat repro.js
var hasFocusStyle = function (attr) {
  if (attr.$ === 4 && attr.b.$ === 11 && !attr.b.a) {
    var _v1 = attr.b;
    var _v2 = _v1.a;
    return true;
  } else {
    return false;
  }
};

hasFocusStyle({ $: 7, a: { $: 2, a: 1 } });

❯ cat repro.js | node -p -
false

❯ uglifyjs repro.js -c | node -p -
false

❯ uglifyjs repro.js -c 'side_effects=false' | node -p -
false

❯ uglifyjs repro.js -c 'pure_getters' | node -p -
false

❯ uglifyjs repro.js -c 'side_effects=false,pure_getters' | node -p -
[stdin]:1
var hasFocusStyle=function(attr){return attr.b.a,4===attr.$&&11===attr.b.$&&!attr.b.a&&!0};hasFocusStyle({$:7,a:{$:2,a:1}});
                                               ^

TypeError: Cannot read properties of undefined (reading 'a')
    at hasFocusStyle ([stdin]:1:48)
    at [stdin]:1:92
    at Script.runInThisContext (node:vm:129:12)
    at Object.runInThisContext (node:vm:307:38)
    at node:internal/process/execution:79:19
    at [stdin]-wrapper:6:22
    at evalScript (node:internal/process/execution:78:60)
    at node:internal/main/eval_stdin:30:5
    at Socket.<anonymous> (node:internal/process/execution:195:5)
    at Socket.emit (node:events:525:35)

Node.js v18.13.0

Final details:

• The input code is generated by Elm.
• pure_getters is safe to use on JS generated by Elm.
• I turn off side_effects because I tried to optimize: I disable everything in uglify that, paired with esbuild's minifier, does not reduce the size of compiled Elm code, in an attempt to minify as quickly as possible.
• I expect the combo side_effects=false,pure_getters not to result in invalid code regardless.

[alexlamsl]

IMHO the generated code is valid − you specified pure_getters=true which tells uglify-js that any property access specified in the original input are all free of side effects (which includes not throwing runtime exceptions). And in this case attr.b.a is clearly not always safe to access, regardless of your assumptions about Elm.

So your best bet in this case is not disabling side_effects.

[lydell]

@alexlamsl I ran git bisect, and the behavior I describe was introduced in #5703. Before that PR, the combo side_effects=false,pure_getters did not result in code with different runtime behavior.

Since it’s something introduced by a PR named “enhance conditionals” it does not feel like an intentional change?

[alexlamsl]

It is intentional in the sense that it is within the definition of pure_getters to allow for such optimisations.

I will look at whether we can avoid generating this intermediate/extraneous attr.b.a which may avoid this particular case − however, there is no guarantee that you won't run into other cases of the same nature.

[lydell]

It might also make sense to update the documentation for pure_getters, because I somehow didn’t realize that is has three values (false, true, strict) until now, and I don’t fully understand the difference between true and strict. I also thought that pure_getters was all about telling UglifyJS that there are no { get some_prop() { launchNukes(); return 5 } } in the code, so it can do more optimization. But it seems to be something more, like (unsafely?) re-ordering code because it can assume something about undefined and null something?

[alexlamsl]

Documentation for pure_getters currently states:

If you pass true for this, UglifyJS will assume that object property access (e.g. foo.bar or foo["bar"]) doesn't have any side effects. Specify "strict" to treat foo.bar as side-effect-free only when foo is certain to not throw, i.e. not null or undefined.

If you would like to improve this text or something else in the documentation, feel free to submit a Pull Request.

[lydell]

If you would like to improve this text or something else in the documentation, feel free to submit a Pull Request.

I would love to, but I don’t understand the option anymore (even when reading that text), so unfortunately I can’t.