closure-compiler icon indicating copy to clipboard operation
closure-compiler copied to clipboard

`Number.toString(base)` does not have `@nosideeffects` semantics

Open KimlikDAO-bot opened this issue 1 year ago • 4 comments

It is mentioned in various places that .toString() method has a hardcoded @nosideeffects property. (e.g., Angular nosideeffect annotation)) (On a side note, is this hack still relevant after the @nosideeffects being allowed in non-externs code?)

However the same is not true for Number.toString(base) variant, which writes the number in bases other than base 10.

// a.js
for (let i = 0; i < 10; ++i)
  i.toString();
for (let j = 0; j < 10; ++j)
  j.toString(16);
google-closure-compiler -O ADVANCED a.js

a.js:2:2: WARNING - [JSC_USELESS_CODE] Suspicious code. This code lacks side-effects. Is there a bug?
  2|   i.toString();
       ^^^^^^^^^^^^

0 error(s), 1 warning(s), 94.7% typed

The first one is detected as suspicious code, the second one isn't.

The relevant externs definition appears correct, however it looks like there some special handling of .toString() in the java code.

This is important because in many fields (crypto, verifiable computation) some constants need to be generated (instead of being written out as literals). If GCC can eliminate the ones that aren't used, it will result in significant reductions. However when the generation code uses toString(16) elimination does not happen.

KimlikDAO-bot avatar Jun 18 '24 09:06 KimlikDAO-bot

We've added this issue to an internal list of possible optimization improvements, but it isn't a high priority.

We would likely accept a pull request.

brad4d avatar Jun 28 '24 19:06 brad4d

Can I take this up ? Can you please assign this to me ?

MadhuriRathod30 avatar Apr 02 '25 16:04 MadhuriRathod30

@MadhuriRathod30

Well, it looks like I can assign it to you, which actually surprised me a bit. Earlier today I was unable to assign a different issue to one of our team members, because we hadn't put them into the appropriate GitHub team yet.

I cannot explain that, but you're welcome. :)

brad4d avatar Apr 02 '25 19:04 brad4d

@brad4d Thank you :-)

MadhuriRathod30 avatar Apr 03 '25 08:04 MadhuriRathod30