Hi Micah, You're right. Quite possible that clang-query counted same function separately for each include in each file. (I was iterating each file separately, but providing all of them at once didn't change the result either.)
It's cool and wrong, so not very useful apparently. :-) Best, Omer On Thu, Oct 24, 2019 at 4:51 PM Micah Kornfield <emkornfi...@gmail.com> wrote: > > 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 <o...@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 <o...@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 <emkornfi...@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 < >> > > fsaintjacq...@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 <anto...@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. >> > > >
