[PATCH] D24686: [support] Some improvements to StringSwitch

[Rui Ueyama via llvm-commits]

On Mon, Oct 3, 2016 at 11:24 AM, Zachary Turner <zturner at google.com> wrote:

>
>
> On Mon, Oct 3, 2016 at 11:16 AM Rui Ueyama <ruiu at google.com> wrote:
>
>> You can instead write this way.
>>
>> auto Ret = StringSwitch<Optional<int>>(Str)
>>     .Case("foo", 1)
>>     ...
>>     .Default(None);
>>
>> instead of
>>
>> auto Ret = StringSwitch<int>(Str)
>>     .Case("foo", 1)
>>     ...
>>     .OrNone();
>>
>> The former pattern is at least as readable as the latter (to me the
>> former is easier to read), so I don't see a strong reason to add a new
>> method to StringSwitch.
>>
>
> Ahh.   I guess the concern I have there is people might not even think to
> do that.  I guess it's obvious in hindsight but I didn't think of using a
> different type than the one I wanted as the argument to StringSwitch.
> Having an OrNone() method makes it kind of explicit in the interface of the
> class that this is supported.
>

I think my concern is that before this patch when you see a line starting
with

  auto Ret = StringSwitch<T>(...

you are sure that Ret is of type T, but with this this can implicitly be
Optional<T> if it ends with OrNone() which I think probably be too subtle.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161003/6454ac9f/attachment.html>