Revert "Save 908,288 bytes by deleting 'const' three times" #14
Conversation
This reverts commit 9a5abb8. VS is still working on a fix to the underlying issue but clang-cl doesn't need this hack so we can remove it. Bug: 934323
|
For full context, I created the original PR in order to work around a VC++ code-gen deficiency. Full details are here: The summary is that const struct/class members confuse VC++ when generating const global objects. It prevents the objects from going in the read-only segment and in this case also cause tons of code to be generated. The original PR was a big win for Chrome. But Chrome doesn't build with VC++ anymore, so this is no longer needed for Chrome. The point of discussion is that the VC++ bug has not yet been fixed. It was forgotten. It's being looked at for VS 2019 (https://twitter.com/TartanLlama/status/1099960433677647874) but, who knows? So, whether to accept this PR comes down to whether VS is still an important customer, and how the project feels about hacky workarounds. It would be possible to #ifdef the changes of course, if that is wanted. So that's where we are. crbug.com/934323 tracks removing these two-year-old hacks from Chrome. |
|
Thanks for the patch. The original patch is still beneficial for those using VC++ build by generating a smaller binary. I think it's worth keeping till VC++ fixes their bug. I'd either keep it or see it ifdef'd. |
|
Makes sense. #ifdefing is ugly but would be a self-documenting way of dealing with it. Alternately I've asked the VC++ team to revisit this when they ship a fix. |
This reverts commit 9a5abb8. VS is
still working on a fix to the underlying issue but clang-cl doesn't need
this hack so we can remove it.
Bug: 934323