backport #42125: Allow constant-propagation to be disabled #42280
Conversation
Our heuristics for constant propagation are imperfect (and probably never will be perfect), and I've now seen many examples of methods that no developer would ask to have const-propped get that treatment. In some cases the cost for latency/precompilation is very large. This renames `@aggressive_constprop` to `@constprop` and allows two settings, `:aggressive` and `:none`. Closes #38983 Co-authored-by: Shuhei Kadowaki <40514306+aviatesk@users.noreply.github.com> Co-authored-by: Martin Holters <martin.holters@hsu-hh.de>
|
This feels a lot like a feature? Why shouldn't it adhere to the feature freeze like the other features? |
|
I thought that was the conclusion in #42188, but reading it again, I might have misunderstood. It does still have the backport label at least. I definitely share the concern, so this may not be a good idea at this point. @timholy @aviatesk Was that the conclusion, or do you still think that PR should be backported? |
|
We don't have to backport the feature, but I'd support backporting the new macro name, because a bunch of packages have already updated to the breakage on master and would need to be un-updated. |
|
I think it may just break Diffractor, for the other packages I made it dependent on |
I think they can just load |
|
Ok. We can fix Diffractor, but it does seem odd to release something with a name that's already been removed on master. I don't feel strongly though. Up to @KristofferC I say, but @simeonschaub maybe you could put up a version that just uses the new macro name, but only supports the aggressive option for 1.7? Should be a two line change. |
|
So basically the thing that's currently defined in Compat? |
|
Aren't there packages that use the old name? |
|
Not anymore, they've all been updated to the new name or commented out. Last vestige I think is mcabbott/TransmuteDims.jl#33, and I didn't check when it was commented out. |
This needed to be backported manually.