[PATCH] Add ability for can_write to indicate why the file is not writable

[Richard Smith]

On Thu, Sep 4, 2014 at 5:19 PM, David Blaikie <dblaikie at gmail.com> wrote:

> On Thu, Sep 4, 2014 at 5:14 PM, Richard Smith <richard at metafoo.co.uk>
> wrote:
>
>> On Thu, Sep 4, 2014 at 5:01 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>> On Wed, Sep 3, 2014 at 6:36 PM, Richard Smith <richard at metafoo.co.uk>
>>> wrote:
>>>
>>>> Hi dblaikie,
>>>>
>>>> Add an optional output parameter to `can_write` to allow it to indicate
>>>> why a file is not considered writable (along with a sample use from Clang
>>>> where we currently produce stupid diagnostics saying `"can't open output
>>>> file: Success"`).
>>>>
>>>> I'm not entirely sure this is the right design, and in particular it's
>>>> a somewhat different interface from the other functions in this file. I
>>>> also considered:
>>>>
>>>>   std::error_code can_write(const Twine &Path, bool &result);
>>>>
>>>> ... which seems redundant because `result` is set to `true` iff the
>>>> error code is `success`, and ...
>>>>
>>>>   std::error_code can_write(const Twine &Path);
>>>>
>>>> ... which is terrible because `bool(can_write(Path))` is `true` iff the
>>>> file is *not* writable, and ...
>>>>
>>>>   std::error_code cannot_write(const Twine &Path);
>>>>
>>>> ... which is inconsistent with other functions near here and ugly.
>>>>
>>>
>>> I'll just put this out there. No idea whether it's sane:
>>>
>>> ErrorOr<void>? (not sure if the ErrorOr template supports void... I
>>> suppose it'd need a whole separate specialization)
>>>
>>> (alternatively, struct Success {}; ErrorOr<Success>)
>>>
>>
>>  ErrorOr<NoneType>? =)
>>
>
> Or that, sure.
>
>
>> I feel a little uncomfortable about using ErrorOr to represent a
>> not-actually-an-error case.
>>
>
> Yeah, I'm not entirely comfortable with it either, and I'm not convinced
> it's the least bad of the options here (nor that it's not the least bad...
> ).
>
> You can't invert it and do bool return with an error_code out parameter
> can you (because error_code isn't default constructible)...
>

You can! error_code default constructs to a success value; bool return with
(optional) error_code out parameter is what the attached patch does.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140904/1eab8291/attachment.html>