randomascii
commented
1 year ago Can you paste the full messages as text? I know they're not selectable in the UI, but it would be worth typing them in or finding them in the source to make it easier to wordsmith them.
Questions: 1) Should the message that this usually happens when defining data in header files? 2) Should the message suggest the possibility of defining the data in source files instead?
Problems with the current advice are: 1) No mention of moving the data from a header file to a source file. 2) It's not clear how changing to constexpr will fix anything, generally speaking. 3) Changing to const won't actually fix anything. If the language is C++ then const implies "static" so the duplicate data will remain. If the language is C then removing the static (leaving just const) will cause duplicate data link errors. 4) __declspec(selectany) does work and is very convenient but it should be acknowledged that it is not part of the C++ standard and not supported by all compilers. It should only be used in C code because... 5) inline data is supported in recent versions of the C++ standard and is a portable way to avoid the duplication.
Describe the bug SizeBench duplicate data advice is inaccurate. Inside the duplicate data feature, SizeBench says that "Changing to const 'constexpr' or 'const' or 'extern declspec(selectany) const' will save copies.". This is only partially true depending on which language the user's project is written in. In Chromium's case, it is written in C++ where both const and constexpr symbols defined inside classes are implied to be static. The only part of the suggestion which works in Chromium's case is 'declspec(selectany) const'.
Expected behavior SizeBench should give advice based on which language the user's project is written in. At the very least, it should have a warning that the current suggestions aren't for C++. This would prevent users from potentially losing development time on C++ projects.
Screenshots SizeBench suggestion after opening duplicate data feature Text: These chunks of data are marked as 'static const' or in some cases just 'const' and have ended up with multiple copies of their data in the binary. Typically you'll see one copy per translation unit referencing the symbol. Changing to 'extern __declspec(selectany) const' will save copies.
SizeBench suggestion after opening a symbol in duplicate data feature Text: This symbol is duplicated between multiple compilands - the most common cause of this is that it is marked 'static const' and would be better off marked as 'const' or 'extern __declspec(selectany) const'. With the way it is defined now, it is wasting space in the binary with the same data in multiple locations.
Environment Details
Additional context N/A