Use uint32_t instead of size_t to store data in basic_specs?

[XZiar]

In recent refactoring, I can see core format spec are now stored in single data_ element, but the type is size_t, which varies in 32bit/64bit mode.

fmt/include/fmt/base.h

Line 742 in 093b39c

size_t data_ = 1 << fill_size_shift;

Since it's not storing pointer information and according to the masks, at most 32bit will be used, is it better to change the type to uint32_t?

fmt/include/fmt/base.h

Lines 705 to 740 in 093b39c

	// Data is arranged as follows:
	//
	// 0 1 2 3
	// 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
	// +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
	// |type |align| w | p | s |u|#|L| f | unused |
	// +-----+-----+---+---+---+-+-+-+-----+---------------------------+
	//
	// w - dynamic width info
	// p - dynamic precision info
	// s - sign
	// u - uppercase (e.g. 'X' for 'x')
	// # - alternate form ('#')
	// L - localized
	// f - fill size
	//
	// Bitfields are not used because of compiler bugs such as gcc bug 61414.
	enum : unsigned {
	type_mask = 0x00007,
	align_mask = 0x00038,
	width_mask = 0x000C0,
	precision_mask = 0x00300,
	sign_mask = 0x00C00,
	uppercase_mask = 0x01000,
	alternate_mask = 0x02000,
	localized_mask = 0x04000,
	fill_size_mask = 0x38000,
	align_shift = 3,
	width_shift = 6,
	precision_shift = 8,
	sign_shift = 10,
	fill_size_shift = 15,
	max_fill_size = 4
	};

[vitaut]

You are right that this doesn't need to be 64-bit but uint32_t is non-portable unfortunately. We could use unsigned long which is normally 32-bit instead, effectively reverting the change to size_t.

[XZiar]

Why is uint32_t non-portable? It should be existing in modern compilers(I believe but happy to know if one is not supportting).
Different compiler would typedef it as different type to match the data width, which is also happening for current size_t.
Using explicit width type seems to be more robust.

The reason for this suggestion is because, when data_ being 64bit, the whole struct get 8B alignment and there will be 4B padding after the fill data. So it actually wastes 8B...

https://en.cppreference.com/w/cpp/language/types#Properties
Under Unix-like OS, long is 8B, and that's just reverting the #4200 .

Actually, I think #4200 is taking about set_fill_size which could do calculation with potential larger datatype.

fmt/include/fmt/base.h

Lines 747 to 749 in 093b39c

	FMT_CONSTEXPR void set_fill_size(size_t size) {
	data_ = (data_ & ~fill_size_mask) | (size << fill_size_shift);
	}

But size should be less than fill_size_mask(and set_fill guaruantees that), don't really see a reason to enlarge data_ decause of it.
For the concern of -Wuseless-cast, pragma ignoredcould help...

[torsten48]

uint32_t was introduced in C++11 (resp. C99), support of theses fixed width types can be safely assumed

[vitaut]

It is not guaranteed to exist by the standard. I don't have specific examples at hand but know that {fmt} is compiled on all kinds of weird platforms, even without intptr_t. That said, we do actually use uint32_t in a few places but generally prefer more portable built-in types. I switched to unsigned in 01914f0 which should have the desired effect in this case. Thanks for the suggestion!

[torsten48]

Vitaut, you are right, std::uint32_t exists only if the platform supports an unsigned integral type that is exactly 32 bits wide and has no padding. It is optional in the standard - I was not aware of this.
But we have std::uint_least32_t, this always exists and is an unsigned integral type with at least 32 bits, and it is the smallest such type.

[vitaut]

std::uint_least32_t is an option but I think unsigned is sufficient.