This is an archive of the discontinued LLVM Phabricator instance.

[AST] Fix double-traversal of code in top-level lambdas in RAV(implicit = yes).
ClosedPublic

Authored by sammccall on Jan 14 2019, 7:01 AM.

Details

Summary

Prior to r351069, lambda classes were traversed or not depending on the
{Function, Class, Namespace, TU} DeclContext containing them.
If it was a function (common case) they were not traversed.
If it was a namespace or TU (top-level lambda) they were traversed as part of
that DeclContext traversal.

r351069 "fixed" RAV to traverse these as part of the LambdaExpr, which is the
right place. But top-level lambdas are now traversed twice.
We fix that as blocks and block captures were apparently fixed in the past.

Maybe it would be nicer to avoid adding the lambda classes to the DeclContext
in the first place, but I can't work out the implications of that.

Diff Detail

Event Timeline

sammccall created this revision.Jan 14 2019, 7:01 AM
sammccall marked an inline comment as done.Jan 14 2019, 7:04 AM
sammccall added inline comments.
unittests/Tooling/RecursiveASTVisitorTests/LambdaExpr.cpp
72

Dirty secret: this is the line that catches the old bug.
The outer lambda was only traversed once (because the *lambdaexpr* was only traversed once) but its body, and thus the inner lambda was traversed twice (causing an assertion).

It'd be possible to express more directly, but it would require hacking up the fixture more, and this new test conveniently tests the lambda-in-lambda-with-VisitImplicitCode case that I forgot in the last patch.

bkramer accepted this revision.Jan 14 2019, 7:53 AM

This makes sense to me.

This revision is now accepted and ready to land.Jan 14 2019, 7:53 AM
This revision was automatically updated to reflect the committed changes.