[PATCH] D140221: [Support] Add StringSwitch::AsOptional

Tomas Matheson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Dec 20 03:28:23 PST 2022


tmatheson added a comment.

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({});




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140221/new/

https://reviews.llvm.org/D140221



More information about the llvm-commits mailing list