Skip to content

Conversation

@johnomotani
Copy link
Contributor

After the discussion on #1835 I was thinking about using an enum class as an option. The way we are using enum classes at the moment has some boiler-plate to define toString and enumFromString functions. Using some magic copied from bout/macro_for_each.hxx, it seems to be possible to replace that with a macro so you can write something like

BOUT_ENUM_CLASS(myenum, foo, bar);

and get an enum class with possible values myenum::foo and myenum::bar, along with toString(myenum e) and myenumFromString(std::string s) functions. I've also put in overloads for Options::as<>() and ostream& operator<< so that the enum class can be loaded from options, e.g.

myenum e = options["myenum_option"].withDefault(myenum::foo);

I'd also like to be able to assign a string to the enum class, but it doesn't seem to be possible to override operator= for an enum class.

Before I think about this any more: is this a useful feature, or an abuse of macros?

Allows creating an 'enum class' along with toString and
<enum name>FromString functions and an Options::as<enum name>() overload
for setting an instance of the enum class from options. Also includes
operator<< overload so that Options::assign() and Options::withDefault()
work.
@johnomotani johnomotani added work in progress Not ready for merging feature proposal A code/feature outline proposal labels Nov 14, 2019
@ZedThree
Copy link
Member

I think this is probably essential! Worthwhile looking over some other implementations of compiletime enum reflection:

Some of these are C++17 only, but could be backported depending on what they use; some make an actual class rather than an enum, which might be painful for the derivative template machinery.
We might be able to use one of those wholesale, or else adapt to our needs.

I've almost finished adding fmt to BOUT++, which could also be useful here. It has fmt::basic_string_view which is an implementation of C++17's std::string_view that some of the above implementations use.

@johnomotani johnomotani added the backport candidiate Does not break backward compatibility, so can be back-ported to v4.4 label Nov 22, 2019
@bendudson
Copy link
Contributor

Thanks @johnomotani I think this is both a wonderful abuse of macros, and a very useful feature. The overload of Options::as is very nice.

bendudson
bendudson previously approved these changes Jan 12, 2020
@d7919
Copy link
Member

d7919 commented Jan 16, 2020

Can the work in progress label be removed?

@johnomotani
Copy link
Contributor Author

Documentation of the new macro is definitely still needed.

@ZedThree - are there any useful changes to make right now from your earlier comment, or do you think it is easy enough to upgrade later?

return found->second; \
} \
\
inline enumname _MAKE_FROMSTRING_NAME(enumname)(std::string s) { \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

std::string s -> const std::string& s

@ZedThree
Copy link
Member

Adding the fmt stuff can be done later once I work out how to do it.

One thing to change now though is that identifiers beginning with an underscore followed by a capital are reserved, so maybe change those -- I suggest just prefixing everything with BOUT_.

@ZedThree
Copy link
Member

ZedThree commented Jan 16, 2020

Also, might need to use this trick to get _BOUT_ENUM_CLASS_MAP_ARGS working on Windows:
cfb7888

But you'll need a Windows system to test it on, so that can wait

@ZedThree ZedThree merged commit 92147c9 into next Feb 4, 2020
@ZedThree ZedThree deleted the bout-enum-class branch February 4, 2020 11:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport candidiate Does not break backward compatibility, so can be back-ported to v4.4 feature proposal A code/feature outline proposal

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants