Add KeywordModifierSet helper for conversion to (likely SemIR) enums #4290
Conversation
toolchain/check/handle_function.cpp
Outdated
| @@ -195,17 +195,14 @@ static auto BuildFunctionDecl(Context& context, | |||
| parent_scope_inst); | |||
| bool is_extern = introducer.modifier_set.HasAnyOf(KeywordModifierSet::Extern); | |||
| SemIR::FunctionFields::VirtualModifier virtual_modifier = | |||
There was a problem hiding this comment.
Note, comparing with the above, maybe SemIR::Function instead of SemIR::FunctionFields here too.
| // Return a builder that implicitly converts to the specified enumeration type | ||
| // mapping the cases passed to the `Case`s specified. For example: | ||
| // ``` | ||
| // SomeEnum e = set.ToEnumerator(SomeEnum::Default) |
There was a problem hiding this comment.
I prefer Enum over Enumerator for brevity, but had you considered an API like StringSwitch:
ToEnum<...>().Case(...).Case(...).Default(...)
This is mainly to make it clearer what the default is. It'd mean making result_ optional and replacing operator T() with auto Default(...) -> T
I think this doesn't even increase verbosity much for call sites because:
SemIR::FunctionFields::VirtualModifier virtual_modifier = modifier_set.ToEnum(...)
becomes:
auto virtual_modifier = modifier_set.ToEnum<SemIR::FunctionFields::VirtualModifier>(...)
(if you're really not a fan of this, you might consider renaming ToEnumerator to ToEnumWithDefault to make it explicit how the argument is used)
There was a problem hiding this comment.
Changed to the shorter ToEnum in ae262bd
I liked not having to explicitly specify the type, though, yeah - if it means using auto on the resulting variable the tradeoff is a bit of a wash. And not having to have an implicit conversion is nice - having Default be the terminating operation makes it clearer I think.
Switched to this in 1b6d5cd
|
I'm happy to have this, now we can bikeshed over API. 😉 |
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
b8f61a7
(based on #4272 (comment))
Could haggle over the name "ToEnum" probably avoids the debate over "enumeration" (the type being returned) v "enumerator" (the value being returned)