Use std::formatted_size() instead of numDigitsAsInt()

[rubiefawn]

Resolves a TODO in lmms_math.h by removing numDigitsAsInt() and using std::formatted_size() instead. Also removes MathTest.cpp, since its only purpose was to test the now removed numDigitsAsInt().

inb4

"Why not just update numDigitsAsInt() to use std::formatted_size() internally?"

numDigitsAsInt() accepts a float, rounds it, casts it to an int, and then calculates the number of digits of that int. The implied use is for cases where a value intended to be an integer is stored as a floating-point value instead. This implicit casting is a communication mistake, and I can imagine a future where somebody tries to use this function by casting their int to a float in order to pass it in. Yuck.

Better to use std::formatted_size() and make the casting explicit at the callsite so that this oddity of storing an integer value in a float is obvious.

Testing

The only place numDigitsAsInt() was used was in Lv2ViewBase.cpp (it seems that pretty much all other LCD spinboxes have hard-coded digit counts) so for testing just load up an LV2 plugin with an integer parameter and make sure it looks identical on this branch vs master.

That said, I'm not sure how much testing is required, given the following short test program passes:

Show test program source

#include <format>
#include <cmath>
#include <cassert>

// The removed function, exactly as it appeared in lmms_math.h
inline int numDigitsAsInt(float f)
{
	// use rounding:
	// LcdSpinBox sometimes uses std::round(), sometimes cast rounding
	// we use rounding to be on the "safe side"
	int asInt = static_cast<int>(std::round(f));
	int digits = 1; // always at least 1
	if(asInt < 0)
	{
		++digits;
		asInt = -asInt;
	}
	// "asInt" is positive from now
	int power = 1;
	for (int i = 1; i < 10; ++i)
	{
		power *= 10;
		if (asInt >= power) { ++digits; } // 2 digits for >=10, 3 for >=100
		else { break; }
	}
	return digits;
}


int main()
{
	// NumDigitsTest() but with assert() instead of QCOMPARE():
	assert(numDigitsAsInt(1.f) == 1);
	assert(numDigitsAsInt(9.9f) == 2);
	assert(numDigitsAsInt(10.f) == 2);
	assert(numDigitsAsInt(0.f) == 1);
	assert(numDigitsAsInt(-100.f) == 4);
	assert(numDigitsAsInt(-99.f) == 3);
	assert(numDigitsAsInt(-0.4f) == 1); // there is no "-0" for LED spinbox
	assert(numDigitsAsInt(-0.99f) == 2);
	assert(numDigitsAsInt(1000000000) == 10);
	assert(numDigitsAsInt(-1000000000) == 11);
	assert(numDigitsAsInt(900000000) == 9);
	assert(numDigitsAsInt(-900000000) == 10);

	// The same, but using std::formatted_size() in place of numDigitsAsInt():
	assert(std::formatted_size("{}", static_cast<int>(std::round(1.f))) == 1);
	assert(std::formatted_size("{}", static_cast<int>(std::round(9.9f))) == 2);
	assert(std::formatted_size("{}", static_cast<int>(std::round(10.f))) == 2);
	assert(std::formatted_size("{}", static_cast<int>(std::round(0.f))) == 1);
	assert(std::formatted_size("{}", static_cast<int>(std::round(-100.f))) == 4);
	assert(std::formatted_size("{}", static_cast<int>(std::round(-99.f))) == 3);
	assert(std::formatted_size("{}", static_cast<int>(std::round(-0.4f))) == 1);
	assert(std::formatted_size("{}", static_cast<int>(std::round(-0.99f))) == 2);
	assert(std::formatted_size("{}", static_cast<int>(std::round(1000000000))) == 10);
	assert(std::formatted_size("{}", static_cast<int>(std::round(-1000000000))) == 11);
	assert(std::formatted_size("{}", static_cast<int>(std::round(900000000))) == 9);
	assert(std::formatted_size("{}", static_cast<int>(std::round(-900000000))) == 10);
}

If that unit test from MathTest.cpp was enough to demonstrate that numDigitsAsInt() was working properly, then perhaps it is enough to show that std::formatted_size() is as well.

[rubiefawn]

Looks like std::format requires GCC 13+, but we still are using GCC 11.4.0, which is causing the linux-x86_64 build to fail. Possibly solved by #7316? (linux-arm64 is using GCC 13.3.0, so it does not have this problem.)

I'm less sure what's going on with Apple Clang. According to their own docs, std::format should be available for "Xcode 15.3 or later", and from the build logs it seems like we use Xcode 16.4 (is this the version of the program, the SDK, or both??) Despite this, it's complaining about std::to_chars() (which is used in std::format) being explicitly unavailable.

I am not familiar with Xcode, so I'm not sure what to look for, but I do wonder if these parts of the logs are related and relevant:

 - Lowest SDK supported by this environment is '10.13' based on /Applications/Xcode_16.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.5.sdk/SDKSettings.plist
- Exporting 'MACOSX_DEPLOYMENT_TARGET=10.13' based on qmake
-- The C compiler identification is AppleClang 17.0.0.17000013
-- The CXX compiler identification is AppleClang 17.0.0.17000013

...

/Applications/Xcode_16.4.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/usr/include/c++/v1/__format/formatter_floating_point.h:67:30: error: 'to_chars' is unavailable: introduced in macOS 13.3 unknown
   67 |   to_chars_result __r = std::to_chars(__first, __last, __value, __fmt);
      |                              ^

Does this mean std::to_chars() requires MACOSX_DEPLOYMENT_TARGET > 10.13? Grrr.

[messmerd]

Looks like std::format requires GCC 13+, but we still are using GCC 11.4.0, which is causing the linux-x86_64 build to fail.

Yeah, this will require GCC 13.

Possibly solved by #7316?

No, that PR only updates the 3rd party libraries, not the compiler.

Unfortunately, AppImages have a minimum glibc version which depends on the GCC compiler version used to build it, so upgrading the Linux x86_64 build runner to Ubuntu 24.04 (which has GCC 13) would also bump the minimum glibc version to 2.39. Because we want the AppImage to be usable on as many distros as possible, we are forced to use an older GCC compiler.

In #8105, we've been exploring a new AppImage runtime which would fix this issue, but it's not ready yet.

Does this mean std::to_chars() requires MACOSX_DEPLOYMENT_TARGET > 10.13? Grrr.

Yes, unfortunately the XCode documentation says the minimum deployment target for std::to_chars/std::from_chars is macOS 10.15 for integral types and macOS 13.3 for floating-point types.

[rubiefawn]

No, that PR only updates the 3rd party libraries, not the compiler.

Ah, I saw "20.04 -> 22.04" and thought "Maybe that has the new GCC version, won't check tho" lol. Good to know.

Oh, I also misinterpreted MACOSX_DEPLOYMENT_TARGET. I assumed the preceding 10. was the 'X' in 'macOS X'. macOS 10.13 is High Sierra from 2017; macOS 13.3 is Ventura from 2022. That's a massive difference in versions.

So I guess I just let this PR age for a decade, until an OS from 2022 starts to feel like a reasonable minimum supported version? 💀

[JohannesLorenz]

The PR looks technically good. Though one note: <format> has 5758 LOC on my system (not counting its 22 includes) and a lot of this is using templates. So I tried to compile your test program, but if-deffed the relevant parts out, once with std::format it took 1.9s, and once with numDigitsAsInt, it took 0.3s.

Luckily, this is just one CPP file affected here. Nonetheless, compile time will increase. I wonder though what our general approach is when it comes to using STL, it seems wrong to just discuss this in one single PR.