constant folding and PolymerRenamer

[maximermilov]

Code snippet from https://github.com/PolymerElements/app-route.git

      observers: [
        '__tryToMatch(route.path, pattern)',
        '__updatePathOnDataChange(data.*)',
        '__tailPathChanged(tail.path)',
        '__routeQueryParamsChanged(route.__queryParams)',
        '__tailQueryParamsChanged(tail.__queryParams)',
        '__queryParamsChanged(queryParams.*)'
      ],

got compiled to

observers:"__tryToMatch(route.path, pattern);__updatePathOnDataChange(data);__tailPathChanged(tail.path);__routeQueryParamsChanged(route.__queryParams);__tailQueryParamsChanged(tail.__queryParams);__queryParamsChanged(queryParams)".split(";")

PolymerRenamer expect observers to be array of strings (to rename function names)

I suggest disabling foldConstants if PolymerPass is enabled.

[MatrixFrog]

cc @robliao

[ChadKillingsworth]

I would love for some pressure to help push forward alternatives to all this string reflection: Polymer/polymer#4543

[ChadKillingsworth]

If you disable foldConstants, it should only be when --polymer_version=1

[maximermilov]

it should only be when --polymer_version=1

why?

[ChadKillingsworth]

Because --polymer_version=2 changes a lot of other semantics. I'm fairly certain you need something more than PolymerRenamer to handle the v2 style conventions and there are other people (myself included) that really want fold constants on with Polymer.

Inside Google, you are going to need to switch to a different approach for renaming polymer elements. Such as https://github.com/Banno/polymer-rename. I think @jart was also working on a solution.

[rictic]

Is there a way to disable constant folding for specific sections of code, or specific files rather than for the whole binary?

[ChadKillingsworth]

Not that I know of. We don't really support avoiding optimizations at micro levels like that. It would be easier if Polymer Renamer recognized this alternate syntax - specifically because that's really the only optimization that can occur on that type of code.

For reference, the optimization won't trigger unless it saves bytes so you'll only see it on longer arrays of strings.

[maximermilov]

Polymer Renamer recognized this alternate syntax

I suspect other optimization can get on the way as well.
In my opinion, safer option would be to teach compiler do not touch such strings.

[robliao]

The Polymer Renamer is essentially destaffed at the moment.

I agree with @ChadKillingsworth that for Polymer v2 the prepass, compile, and postpass is the way to go until Polymer stops using string based expressions.

Having said that, I'm happy to review a pull request to the Polymer Renamer.

[ChadKillingsworth]

If you really want to address this properly, then we need to add the ability to parse observers and paths and emit the appropriate reflection calls to the PolymerPass. Someone would just need to add that ability.

Doing that would enable us to also add the necessary sink calls to prevent dead code elimination of the methods.

I'd be happy to assist or review and test the changes.

cc @azakus

[dimvar]

@maximermilov in the internal version of the compiler, you can enable/disable passes individually, so you can disable constant folding if you wish.

As Chad said, the changes to avoid this issue should happen in Polymer Renamer and/or the Polymer pass in the compiler, not in the optimizations.

[dimvar]

Chad tells me that Polymer Renamer is also incompatible with the compiler's type-based optimizations, so it sounds like some more work is needed to make Polymer Renamer compatible with advanced mode.