[PATCH] D34052: [XRay][clang] Support capturing the implicit `this` argument to C++ class member functions
Dean Michael Berris via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jun 13 00:28:12 PDT 2017
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?
More information about the cfe-commits