[PATCH] D33895: [Support] Add TempFailureRetry helper function

[Pavel Labath via Phabricator via llvm-commits]

labath reopened this revision.
labath added inline comments.
This revision is now accepted and ready to land.


================
Comment at: include/llvm/Support/Errno.h:32
 
+template <typename Fun, typename... Args>
+inline typename std::result_of<Fun && (Args && ...)>::type
----------------
labath wrote:
> EricWF wrote:
> > The value-categories seem all messed up here. There is no reason to be using `&&` anywhere, since all of the arguments and the function are lvalues. Personally I would write this function as:
> > 
> > ```
> > template <typename Fun, typename... Args,
> >           typename ResultT = typename std::result_of<Fun const& (const Args&...)>::type>
> > inline ResultT RetryAfterSignal(ResultT Fail, const Fun &F, const Args &... As) {
> >   ResultT Res;
> >   do
> >     Res = F(As...);
> >   while (Res == Fail && errno == EINTR);
> >   return Res;
> > }
> > ```
> Thanks, that looks much better. I'll have to remember the extra template arg trick.
> 
> Using the correct value category is still somewhat of a mystery to me.
When I started using this function, I noticed that the ResultT trick did not work after all, because the template argument deduction would override the default template value. My attempt to fix that in r306003 uncovered another issue -- libc++'s implementation of std::result_of does not handle the prototype of open(2) correctly (it's a variadic function), so I've reverted this patch until that is sorted.

At this point I don't see any other solution than to go back to the initial implementation of having a separate `FailT` template argument, and using decltype instead of std::result_of. Do you have a better idea?


Repository:
  rL LLVM

https://reviews.llvm.org/D33895