This is an archive of the discontinued LLVM Phabricator instance.

[Support] Add StringSwitch::AsOptional
AbandonedPublic

Authored by lenary on Dec 16 2022, 8:03 AM.

Details

Summary

A StringSwitch does not always have a default value. In these cases, you
can currently achieve the same using the following, which is pretty
ugly:

std::optional<InnerT> Foo = llvm::StringSwitch<std::optional<InnerT>>(str)
  .Case("foo", std::optional(InnerT(...)))
  .Default(std::nullopt);

This commit allows you to achieve the same using the following, which I
think makes the intent a lot clearer (and avoids a nested optional
internally):

std::optional<InnerT> Foo = llvm::StringSwitch<InnerT>(str)
  .Case("foo", InnerT(...))
  .AsOptional();

Diff Detail

Event Timeline

lenary created this revision.Dec 16 2022, 8:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2022, 8:03 AM
lenary requested review of this revision.Dec 16 2022, 8:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 16 2022, 8:03 AM
barannikov88 added inline comments.
llvm/include/llvm/ADT/StringSwitch.h
189–192

Would that work? Note that Result is std::optional<T> and std::optional has a converting move constructor.

lenary added inline comments.Dec 16 2022, 12:46 PM
llvm/include/llvm/ADT/StringSwitch.h
189–192

Maybe. I'm not super familiar with move semantics, but i can see how that should work based on the description at cppreference.com, and I like that it's simpler. I'll try it with the follow-on patch that uses it.

I'm not convinced this gives us much over using InnerT=optional directly; arguably it just obfuscates the type. There are no examples in the codebase for StringSwitch<std::optional>, but there are about 80 of StringSwitch<llvm::Optional>, and they typically look like this:

auto Level = llvm::StringSwitch<Optional<Driver::ReproLevel>>(A->getValue())
                 .Case("off", Driver::ReproLevel::Off)
                 .Case("crash", Driver::ReproLevel::OnCrash)
                 .Case("error", Driver::ReproLevel::OnError)
                 .Case("always", Driver::ReproLevel::Always)
                 .Default(None);

The std::optional equivalent can be written pretty much the same. In your before example, you don't need explicit std::optional constructor calls, you can use auto because they type is obvious from the RHS, and you can use the default constructor for std::optional (debatable whether that is preferable to std::nullopt):

auto Foo = llvm::StringSwitch<std::optional<InnerT>>(str)
    .Case("foo", InnerT(...))
    .Default({});
lenary abandoned this revision.Dec 20 2022, 4:10 AM

Ah, ok, yeah that is simpler.