solidity icon indicating copy to clipboard operation
solidity copied to clipboard

Published 20 hours ago • verified publisher icon ethereum

Document UnusedStoreEliminator

Open nikola-matic opened this issue 3 years ago • 2 comments

Addresses #13497

  • [x] Update code comment in UnusedStoreEliminator.h
  • [x] Check other undocumented optimizer steps

nikola-matic avatar Sep 08 '22 11:09 nikola-matic

@ekpyron can you remove the change request so some one else can take over the review, as I'd rather not have this sit around for another couple of weeks :)

nikola-matic avatar Sep 19 '22 12:09 nikola-matic

You can just dismiss the review :)

But in any case, I'm now back from Berlin and catching up so I'm going to go back to this pretty soon.

cameel avatar Sep 19 '22 15:09 cameel

Addresses #13497

Does it fix the issue completely?

cameel avatar Oct 20 '22 18:10 cameel

Addresses #13497

Does it fix the issue completely?

I believe so, yes.

nikola-matic avatar Oct 20 '22 18:10 nikola-matic

OK, changed to Fixes #13497 to have github close it automatically.

cameel avatar Oct 20 '22 18:10 cameel

Tentatively approving, but please squash the review fixes and I still think the index entry would be better off pointing at the section that is about the keyword, not the whole page

My issue with this is that there's no uniformity regarding this in optimizer.rst if I make this change - our current top level index:: doesn't have the vast majority of the optimizer steps in there anyway, and there's not a single usage of a ! main entry topic either. I do agree that your suggestion would make things better, but only if applied to the whole document. I'm gonna squash the commits, and open a separate issue for this.

nikola-matic avatar Oct 20 '22 20:10 nikola-matic