[PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions

[David Blaikie via cfe-commits]

hard to say if it's more readable without seeing it - if you could attach a
patch, if it's easy enough/you worked it up before, might be worth
comparing/contrasting


On Tue, Jun 13, 2017 at 12:28 AM Dean Michael Berris via Phabricator <
reviews at reviews.llvm.org> wrote:

> dberris added a comment.
>
> In https://reviews.llvm.org/D34052#778670, @dblaikie wrote:
>
> > I take it there's already handling for these attributes on non-member
> functions?
>
>
> Yes, we're just extending it to also apply to the case where it doesn't
> support this one case where we actually do need the implicit this argument
>
> > There's probably a reason this code can't be wherever that code is &
> subtract one from the index? (reducing code duplication by having all the
> error handling, etc, in one place/once)
>
> I tried doing it for the `checkFunctionOrMethodNumParams` function, but it
> ended up being much more complicated. :(
>
> The choice is between adding a bool argument (e.g. AllowImplicitThis) in
> `checkFunctionOrMethodParameterIndex(...)` that's default to always true
> (to preserve existing behaviour) but the checks and implementation of that
> template has a number of assumptions as to whether the index is valid for
> member functions.
>
> I can change this so that the logic is contained in
> `checkFunctionOrMethodParameterIndex(...)` if that's more readable?
>
>
> https://reviews.llvm.org/D34052
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170613/e5543863/attachment.html>