arrow-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Micah Kornfield <emkornfi...@gmail.com>
Subject Re: [DISCUSS] Result vs Status
Date Thu, 24 Oct 2019 21:50:58 GMT
Hi Omer,
I think this is really cool.  It is quite possible it was underestimated (I
agree about line lengths), but I think the clang query is double counting
somehow.

For instance:

"grep -r Status *" only returns ~9000 results in total for me.

Similarly using grep for "FinishTyped" returns 18 results for me.
Searching through the log that you linked seems to return 450 (for "Status
FinishTyped").

It is quite possible, I'm doing something naive with grep.

Thanks,
Micah

On Thu, Oct 24, 2019 at 2:41 PM Omer F. Ozarslan <omer@utdallas.edu> wrote:

> Forgot to mention most of those lines are longer than line width while
> out is usually (always?) last parameter, so probably that's why grep
> possibly underestimates their number.
>
> On Thu, Oct 24, 2019 at 4:33 PM Omer F. Ozarslan <omer@utdallas.edu>
> wrote:
> >
> > Hi,
> >
> > I don't have much experience on customized clang-tidy plugins, but
> > this might be a good use case for such a plugin from what I read here
> > and there (frankly this was a good excuse for me to have a look at
> > clang tooling as well). I wanted to ensure it isn't obviously overkill
> > before this suggestion: Running a clang query which lists functions
> > returning `arrow::Status` and taking a pointer parameter named `out`
> > showed that there are 13947 such functions in `cpp/src/**/*.h`. [1]
> >
> > I checked logs and it seemed legitimate to me, but please check it in
> > case I missed something. If that's the case, it might be tedious to do
> > this work manually.
> >
> > [1]: https://gist.github.com/ozars/ecbb1b8acd4a57ba4721c1965f83f342
> > (Note that the log file is shown as truncated by github after ~30k
> > lines)
> >
> > Best,
> > Omer
> >
> >
> >
> > On Wed, Oct 23, 2019 at 9:23 PM Micah Kornfield <emkornfield@gmail.com>
> wrote:
> > >
> > > OK, it sounds like people want Result<T> (at least in some
> circumstances).
> > > Any thoughts on migrating old APIs and what to do for new APIs going
> > > forward?
> > >
> > > A very rough approximation [1] yields the following counts by module:
> > >
> > >  853 arrow
> > >
> > >   17 gandiva
> > >
> > >   25 parquet
> > >
> > >   50 plasma
> > >
> > >
> > >
> > > [1] grep -r Status cpp/src/* |grep ".h:" | grep "\\*" |grep -v Accept
> |sed
> > > s/:.*// | cut -f3 -d/ |sort
> > >
> > >
> > > Thanks,
> > >
> > > Micah
> > >
> > >
> > >
> > > On Sat, Oct 19, 2019 at 7:50 PM Francois Saint-Jacques <
> > > fsaintjacques@gmail.com> wrote:
> > >
> > > > As mentioned, Result<T> is an improvement for function which returns
> a
> > > > single value, e.g. Make/Factory-like. My vote goes Result<T> for
such
> > > > case. For multiple return types, we have std::tuple like Antoine
> > > > proposed.
> > > >
> > > > François
> > > >
> > > > On Fri, Oct 18, 2019 at 9:19 PM Antoine Pitrou <antoine@python.org>
> wrote:
> > > > >
> > > > >
> > > > > Le 18/10/2019 à 20:58, Wes McKinney a écrit :
> > > > > > I'm definitely uncomfortable with the idea of deprecating Status.
> > > > > >
> > > > > > We have a few kinds of functions that can fail:
> > > > > >
> > > > > > 1. Functions with no "out" arguments
> > > > > > 2. Functions with one out argument
> > > > > > 3. Functions with multiple out arguments
> > > > > >
> > > > > > IMHO functions in category 2 are the best candidates for
> utilizing
> > > > > > Status. In some cases, Case 3 may be more usable Result-based,
> but it
> > > > > > can also create more work (or confusion) on the part of the
> developer,
> > > > > > either
> > > > > >
> > > > > > * The T in Result<T> has to be a struct-like value that
> transports
> > > > > > multiple pieces of data
> > > > >
> > > > > The T can be a std::tuple though, so you need not necessarily
> define a
> > > > > dedicated struct type for a single API's return value.
> > > > >
> > > > >  > Can't say I'm thrilled about having Result<void> or similar
for
> Case
> > > > >  > 1-type functions (if I'm understanding what would be the
> solution
> > > > >  > there).
> > > > >
> > > > > Agreed.
> > > > >
> > > > > Regards
> > > > >
> > > > > Antoine.
> > > >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message