impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jim Apple <jbap...@cloudera.com>
Subject Re: Preferred syntax for warning about ignored Status returns
Date Mon, 09 Jan 2017 18:00:14 GMT
That's applying it to the type definition. At the type use:

https://godbolt.org/g/RMYVW7

On Mon, Jan 9, 2017 at 9:56 AM, Zachary Amsden <zamsden@cloudera.com> wrote:
> GCC doesn't catch this when optimization is enabled and the result is
> discarded:
>
> https://godbolt.org/g/4b0BQC
>
> I think that means a type wrapper approach is needed, which probably
> necessitates option 1.
>
>  - Zach
>
> On Mon, Jan 9, 2017 at 9:17 AM, Jim Apple <jbapple@cloudera.com> wrote:
>
>> My vote, as I mentioned on the patch, is option 1. I see MUST_USE(T)
>> as a property of T, like const T or volatile T. I think it is dual to
>> move semantics or to Rust's ability to temporarily or permanently
>> consume values so that /only/ one copy is in use rather than
>> MUST_USE's /at least one/.
>>
>> https://en.wikipedia.org/wiki/Substructural_type_system
>>
>> On Fri, Jan 6, 2017 at 4:05 PM, Taras Bobrovytsky
>> <tbobrovytsky@cloudera.com> wrote:
>> > I'd vote for #2. I think it's better to have less important information
>> > (such as qualifiers) towards the end of lines. (I think it would be nice
>> if
>> > modifiers such as public and private were at the end of method
>> declarations
>> > in Java, for example: void myMethod() private static {...})
>> >
>> > On Fri, Jan 6, 2017 at 3:38 PM, Daniel Hecht <dhecht@cloudera.com>
>> wrote:
>> >
>> >> As I indicated in the original review, my preference is #2 but I don't
>> feel
>> >> too strongly.
>> >>
>> >> On Fri, Jan 6, 2017 at 2:59 PM, Tim Armstrong <tarmstrong@cloudera.com>
>> >> wrote:
>> >>
>> >> > Hi All,
>> >> >   I wanted to poll the Impala community for opinions about style for
>> >> > declaring functions where the caller is expected to do something with
>> the
>> >> > return value.
>> >> >
>> >> > Ideally we'd be able to declare Status with an attribute that made
>> this
>> >> > take effect globally, but unfortunately that's not available until
>> C++17.
>> >> >
>> >> > So we need to annotate each Status-returning function. The two
>> >> alternatives
>> >> > we discussed on this CR (https://gerrit.cloudera.org/#/c/4878/) were:
>> >> >
>> >> > #1 - a special macro wrapping Status
>> >> >
>> >> > MUST_USE(Status) DoSomethingThatCanFail(int64_t foo, Bar* bar);
>> >> >
>> >> > Pros:
>> >> > * Closely connected to the return type that it affects
>> >> > * It's easier to search/replace Status with MUST_USE(Status)
>> >> >
>> >> > Cons:
>> >> > * Could get visually noisy if we use it everywhere
>> >> >
>> >> > #2 - a macro that gets appended to the declaration:
>> >> >
>> >> > Status DoSomethingThatCanFail(int64_t foo, Bar* bar)
>> WARN_UNUSED_RESULT;
>> >> >
>> >> > Pros:
>> >> > * Macro is slightly
>> >> > * Less visually noisy since it's at the end of the declaration
>> >> >
>> >> > What do people think?
>> >> >
>> >>
>>

Mime
View raw message