Neargye / magic_enum

Static reflection for enums (to string, from string, iteration) for modern C++, work with any enum type without any macro or boilerplate code
MIT License
4.74k stars 422 forks source link

Suggestion: consider using C++20 `source_location` utilities #292

Open SainoNamkho opened 1 year ago

SainoNamkho commented 1 year ago

C++20 now has a standard way to get __PRETTY_FUNCTION__. This may ease the effort to work with compilers.

In addition, I found the hard coded offset has changed during the commits, reflecting changes of the implementations, would you mind documenting a brief naming rule among different versions of these implementations? And I've tested this approach without magic numbers, will it be an alternative?

Neargye commented 11 months ago

Do you know if this gives some kind of boost to compilation time?

Epixu commented 6 months ago

The provided approach had bad corner cases where it cropped names wrongly, if they ended up with similar characters, so I took the liberty to improve on the snippet.

When it comes to compile speed - it didn't seem to affect my reflection tests, but is definitely a huge improvement when it comes to future maintenance - compilers are many and they often change. Keeping track of the proper offsets will be a chore, it is much more elegant to automate it by using this handsome utility.

Epixu commented 6 months ago

~Did some more tinkering, trying to implement that mechanism on a working code base, and GCC/Clang gave me a lot of grief. Only Clang-Cl 16.0.6 reports bad function_name() output, while GNU 11.4.0 compiles, but segfaults at a consistent, but unexplicable place.~

~Smells like bad standard library implementation. Only latest MSVC seems to not cause any problems whatsoever. Unfortunately, I'm completely unable to reproduce this in godbolt for some reason...~

~So I'm definitely reverting my use of this, and it should be extensively tested, if anyone attempts it. Hopefully it's not just some ridiculous mistake on my behalf.~

EDIT: Nope, it's my mistake - turns out that under GCC/Clang, due to the way the text is formatted, it is possible to have a corner case, where left side of the string can also match the pattern i provided.

Here's that accounted for

Additionally, the same strategy can be applied with the conventional way its done up until now, just substitute source_location::current().function_name() with REFLECT_FUNCTION() as defined here:

/// Shamelessly stolen from boost and extended to my liking                   
/// Dumps the current function name                                           
#if defined(__GNUC__) or (defined(__MWERKS__) and (__MWERKS__ >= 0x3000)) or (defined(__ICC) and (__ICC >= 600)) or defined(__ghs__)
   #define REFLECT_FUNCTION() __PRETTY_FUNCTION__
#elif defined(__clang__)
   #define REFLECT_FUNCTION() __PRETTY_FUNCTION__
#elif defined(__DMC__) and (__DMC__ >= 0x810)
   #define REFLECT_FUNCTION() __PRETTY_FUNCTION__
#elif defined(__FUNCSIG__) or defined(_MSC_VER)
   #define REFLECT_FUNCTION() __FUNCSIG__
#elif (defined(__INTEL_COMPILER) and (__INTEL_COMPILER >= 600)) or (defined(__IBMCPP__) and (__IBMCPP__ >= 500))
   #define REFLECT_FUNCTION() __FUNCTION__
#elif defined(__BORLANDC__) and (__BORLANDC__ >= 0x550)
   #define REFLECT_FUNCTION() __FUNC__
#elif defined(__STDC_VERSION__) and (__STDC_VERSION__ >= 199901)
   #define REFLECT_FUNCTION() __func__
#elif defined(__cplusplus) and (__cplusplus >= 201103)
   #define REFLECT_FUNCTION() __func__
#else
   #error Not implemented
#endif

and voila, automatic indices

frederick-vs-ja commented 2 months ago

std::source_location::function_name is not a standard way to get __PRETTY_FUNCTION__. It is allowed, but not required, to give more information than __func__. Currently, only implementations guarantee ways to obtain detailed function signatures, and we should continue to use what the implementations guarantee.

I personally suggest to use std::source_location::current().function_name() as a fallback when neither __PRETTY_FUNCTION__ nor __FUNCSIG__ is usable. However, it's doubtful where there is, or will be, even one implementation where only these standand functions give guaranteed and wanted results.