[PATCH] Add a hook RecursiveASTVisitor::TraverseLambdaBody.

[James Dennett]

On Tue, Jul 9, 2013 at 10:12 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> On Tue, Jul 9, 2013 at 10:08 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
>> On Tue, Jul 9, 2013 at 8:01 PM, James Dennett <jdennett at google.com> wrote:
>>> jdennett added you to the CC list for the revision "Add a hook RecursiveASTVisitor::TraverseLambdaBody.".
>>>
>>> Hi rsmith,
>>>
>>> Add a hook RecursiveASTVisitor::TraverseLambdaBody, to enable visitors to use/maintain additional state from the LambdaExpr while visiting the body of a LambdaExpr.
>>>
>>> One use for this arises because Clang's AST currently holds lambda bodies in a form prior to their adjustment to refer to captured copies of local variables, and so some clients will need access to the lambda's closure type in order to query how to map VarDecl*s to the FieldDecls of their by-copy captures.  This hook is sufficient for at least one such client; to do this without such a hook would require the client to re-implement the whole of TraverseLambdaExpr, which is non-trivial and would likely be more brittle.
>>
>> I'm not sure I see the point; you can already override
>> TraverseLambdaCapture, or override TraverseLambdaExpr and have your
>> implementation call into the base class implementation.
>
> Err, wait, nevermind, not thinking; I withdraw my objection.

If there's something I could improve in the CL description to make it
require less thinking, let me know.

> Please change the test so it's separate from the SourceLocation test;
> mixing them together might be slightly shorter, but it makes the
> purpose of the test much less clear.

Done, thanks for the suggestion.  (Shorter is only good when it has
other benefits.  Here, not so much.)

It would be possible to separate even more, by breaking out a separate
visitor for just this one test.  My feeling is that separating the
test itself out is sufficient, but I'm open to changing it if anyone
feels strongly.

(Richard "accepted" this patch via Phabricator, but I'm holding out
for a more explicit go-ahead before committing.)