[Lldb-commits] [PATCH] D77759: [lldb/Reproducers] Fix passive replay for functions returning string as (char*, size_t).

Tue Apr 21 03:12:43 PDT 2020

labath added a comment.

In D77759#1992670 <https://reviews.llvm.org/D77759#1992670>, @JDevlieghere wrote:

> In D77759#1992252 <https://reviews.llvm.org/D77759#1992252>, @labath wrote:
>
> > In D77759#1988419 <https://reviews.llvm.org/D77759#1988419>, @labath wrote:
> >
> > > Hmm... well, I like how this (active&passive) is unified at the template level. It still feels like there's too much code to implement this thing (coming from the duplication for const, non-const and static variants), but I'm not sure what can be done about that. I'll need to ponder this a bit more
> >
> >
> > After sleeping on this a couple of times, I've come to believe that the problem here is that we've let the redirection functions inherit the c++ function pointer weirdness, which resulted in a need to write template goo for const, non-const and static methods (and maybe sometimes even constructors). The goo is (more or less) unavoidable if we want to have compiler-generated "identifiers" for all these functions, but there's a difference between writing that once, and once for each redirection we need to make. Particularly as it doesn't seem like there's a compelling reason for that. There no reason that the "redirected" function in the replayer machinery needs be globally unique in the same way as the "original" functions do. In fact I think it doesn't even have to be a template at all -- it could just take the function-to-wrap as a regular argument instead of a templated one.
>
>
> Yeah, I've come to the same conclusion while working on this. Hindsight is 20/20 and changing all that might be a significant amount of work.

That last part makes me thing that we have not come to the same conclusion. :P I'm not talking about changing the entire mechanism of how we assign "ids" to functions and stuff. While it is true that it's not working out as well as I originally hoped, I still think that's ok, and having a bit of template goo to enable that is not bad. I'm just talking about changing how the "redirection" mechanism works -- the redirected functions don't need to be uniquely identifiable in the same way, as they can just be linked to the original function id, and so we should be able to come up with a less goo-ey mechanism for writing those. While that would certainly be "work" I don't think it would be a "significant" amount of work, as we only have a handful of redirected functions.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77759/new/

https://reviews.llvm.org/D77759