It doesn't seem like there's a clear consensus - I counted two people in favour of Option 1 and three in favour of Option 2 (one weakly).
I'll probably go forward with Option 2 because other projects that we (now or in future) pull in source from (Kudu, gutil) use it. On Mon, Jan 9, 2017 at 10:33 AM, Zachary Amsden <[email protected]> wrote: > Yes Jim, exactly. I also sent the wrong link at least once, but this was > my point. In the absence of compiler support I think it is actually > possible to implement this anyway using syntax 1 but that is left as an > exercise to the reader. > > On Mon, Jan 9, 2017 at 10:20 AM, Jim Apple <[email protected]> wrote: > > > Though the attribute is "on" the definition, it can't appear in that > > location (after parameters, before curly braces). I don't know why. > > > > I think I now understand what you were saying above: if we use > > WARN_UNUSED_RESULT, then it can go after the function for functions > > with prototypes but must go earlier in the line for functions without > > prototypes. > > > > Did I get that right? > > > > On Mon, Jan 9, 2017 at 10:11 AM, Zachary Amsden <[email protected]> > > wrote: > > > Maybe I'm just being dense but I couldn't get it to work with syntax 2 > > on a > > > function definition without having a separate forward declaration: > > > > > > https://godbolt.org/g/ODtoQC vs. https://godbolt.org/g/WCxDZv > > > (non-functional) > > > > > > - Zach > > > > > > On Mon, Jan 9, 2017 at 10:00 AM, Jim Apple <[email protected]> > wrote: > > > > > >> 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 <[email protected]> > > >> 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 <[email protected]> > > 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 > > >> >> <[email protected]> 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 < > [email protected]> > > >> >> 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 < > > >> [email protected]> > > >> >> >> 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? > > >> >> >> > > > >> >> >> > > >> >> > > >> > > >
