impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Zachary Amsden <zams...@cloudera.com>
Subject Re: Preferred syntax for warning about ignored Status returns
Date Mon, 09 Jan 2017 17:56:05 GMT
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
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message