HomePhabricator

Make RecursiveASTVisitor visit lambda capture initialization expressions

Description

Make RecursiveASTVisitor visit lambda capture initialization expressions

Summary:
Lambda capture initializations are part of the explicit source code and
therefore should be visited by default but, so far, RecursiveASTVisitor does not
visit them.

This appears to be an oversight. Because the lambda body needs custom handling
(calling TraverseLambdaBody()), the DEF_TRAVERSE_STMT for LambdaExpr sets
ShouldVisitChildren to false but then neglects to visit the lambda capture
initializations. This patch adds code to visit the expressions associated with
lambda capture initializations.

Reviewers: klimek

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D22566

Event Timeline

I don't understand the intent of this patch.

What are "lambda capture initialization expressions"? An example would be informative; Clang doesn't really document what it means by this anywhere that I've found, and it's not a standard term. The test case shows a plain old value capture. C++14 has init-captures, which have initialization expressions, but C++11's captures don't have initialization expressions in the source code, just names.

In any case I suspect that the right place to handle this might be in TraverseLambdaCapture, which currently does nothing except for init-captures. Doing it anywhere else runs the risk of multiple visitations of init captures, and of visiting implicit captures.

mboehme added a comment.EditedAug 5 2016, 3:04 AM

What are "lambda capture initialization expressions"? An example would be informative; Clang doesn't really document what it means by this anywhere that I've found, and it's not a standard term.

Sorry, I assumed it was.

What I mean are the Exprs in LambdaExpr::capture_inits() (which are also children of the LambdaExpr, along with the CompoundStmt that is the lambda body).

(BTW, I've noticed now that even LambdaExpr itself can't decide what to call these Exprs -- the documentation for capture_inits() calls them "initialization expressions", while capture_init_begin() a few lines below calls them "initialization arguments". I'll call them "initialization expressions" in the following.)

The test case shows a plain old value capture. C++14 has init-captures, which have initialization expressions, but C++11's captures don't have initialization expressions in the source code, just names.

The reason I want to visit the initialization expressions is because I'm working on a check that wants to find all uses of a given variable -- in the sense of all DeclRefExprs that refer to that variable.

Even for a plain value capture, the variable is used (in the sense that a DeclRefExpr to it is evaluated) at the time that the lambda is created -- to create the copy of that variable that is stored in the lambda. It's important for my check to find this use and not only the uses of the variable inside the lambda itself; those uses are evaluated at a different time and, arguably, aren't even uses of the original variable (but of the copy of the variable that lives inside the lambda).

I would argue that the use of the variable that happens when the lambda is created is an explicit use, since the variable is explicitly named in the capture list, though for my purposes it would also work if we visited that use only when shouldVisitImplicitCode() returns true -- see also the discussion of implicit captures below.

In any case I suspect that the right place to handle this might be in TraverseLambdaCapture, which currently does nothing except for init-captures. Doing it anywhere else runs the risk of multiple visitations of init captures, and of visiting implicit captures.

Thanks for pointing this out to me -- I've looked at my patch again, and it looks to me as if there are issues with the way it handles both of these cases:

a) Init captures are indeed visited twice: Once in the loop that my patch adds, and then again when TraverseLambdaCapture() calls TraverseDecl() on the variable that is effectively declared by the init capture.

b) Implicit captures (or, more accurately, their initialization expressions) are currently visited even if shouldVisitImplicitCode() returns false. This obviously should not happen (though I would argue they should be visited if shouldVisitImplicitCode() returns true).

I'll prepare a patch that fixes these issues -- probably in TraverseLambdaCapture().

One issue I'll have to deal with is that TraverseLambdaCapture() doesn't have direct access to the initialization expression for the capture. (For init captures, the initialization expression can be accessed through C->getCapturedVar()->getInit(), but for non-init-captures, this doesn't work -- it returns the Expr that was used to initialize the variable when it was originally declared.)

Instead, I'm hoping it's guaranteed that there's a 1:1 correspondence between the entries in LambdaExpr::captures() and LambdaExpr::capture_inits(). (Do you know if this is the case?) That would then allow me to find the initialization expression for a capture C as follows:

const Expr *InitExpr = LE->capture_init_begin() +
    (C - LE->capture_begin());

A patch that fixes the issues is at D23204.