Track lifetime for lambdas with explicit capture (#2776)

This commit is contained in:
Paul Fultz II 2020-09-05 00:56:01 -05:00 committed by GitHub
parent 4738da3d69
commit cc2bc74084
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 124 additions and 22 deletions

View File

@ -299,6 +299,10 @@ static T* nextAfterAstRightmostLeafGeneric(T* tok)
if (!rightmostLeaf || !rightmostLeaf->astOperand1())
return nullptr;
do {
if (const Token* lam = findLambdaEndToken(rightmostLeaf)) {
rightmostLeaf = lam;
break;
}
if (rightmostLeaf->astOperand2())
rightmostLeaf = rightmostLeaf->astOperand2();
else

View File

@ -911,6 +911,16 @@ static void compilePrecedence2(Token *&tok, AST_state& state)
// - Compile the content of the lambda function as separate tree (this is done later)
// this must be consistent with isLambdaCaptureList
Token* const squareBracket = tok;
// Parse arguments in the capture list
if (tok->strAt(1) != "]") {
Token* tok2 = tok->next();
AST_state state2(state.cpp);
compileExpression(tok2, state2);
if (!state2.op.empty()) {
squareBracket->astOperand2(state2.op.top());
}
}
if (Token::simpleMatch(squareBracket->link(), "] (")) {
Token* const roundBracket = squareBracket->link()->next();
Token* curlyBracket = roundBracket->link()->next();

View File

@ -3706,8 +3706,12 @@ static void valueFlowLifetimeConstructor(Token* tok, TokenList* tokenlist, Error
}
struct Lambda {
enum class Capture {
ByValue,
ByReference
};
explicit Lambda(const Token * tok)
: capture(nullptr), arguments(nullptr), returnTok(nullptr), bodyTok(nullptr) {
: capture(nullptr), arguments(nullptr), returnTok(nullptr), bodyTok(nullptr), explicitCaptures() {
if (!Token::simpleMatch(tok, "[") || !tok->link())
return;
capture = tok;
@ -3722,12 +3726,31 @@ struct Lambda {
} else if (Token::simpleMatch(afterArguments, "{")) {
bodyTok = afterArguments;
}
for(const Token* c:getCaptures()) {
if (c->variable()) {
explicitCaptures[c->variable()] = std::make_pair(c, Capture::ByValue);
} else if (c->isUnaryOp("&") && Token::Match(c->astOperand1(), "%var%")) {
explicitCaptures[c->astOperand1()->variable()] = std::make_pair(c->astOperand1(), Capture::ByReference);
} else {
const std::string& s = c->expressionString();
if (s == "=")
implicitCapture = Capture::ByValue;
else if (s == "&")
implicitCapture = Capture::ByReference;
}
}
}
const Token * capture;
const Token * arguments;
const Token * returnTok;
const Token * bodyTok;
std::unordered_map<const Variable*, std::pair<const Token*, Capture>> explicitCaptures;
Capture implicitCapture;
std::vector<const Token*> getCaptures() {
return getArguments(capture);
}
bool isLambda() const {
return capture && bodyTok;
@ -3762,11 +3785,15 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
const Scope * bodyScope = lam.bodyTok->scope();
std::set<const Scope *> scopes;
// Avoid capturing a variable twice
std::set<nonneg int> varids;
auto isCapturingVariable = [&](const Token *varTok) {
auto isImplicitCapturingVariable = [&](const Token *varTok) {
const Variable *var = varTok->variable();
if (!var)
return false;
if (varids.count(var->declarationId()) > 0)
return false;
if (!var->isLocal() && !var->isArgument())
return false;
const Scope *scope = var->scope();
@ -3777,23 +3804,39 @@ static void valueFlowLifetime(TokenList *tokenlist, SymbolDatabase*, ErrorLogger
if (scope->isNestedIn(bodyScope))
return false;
scopes.insert(scope);
varids.insert(var->declarationId());
return true;
};
// TODO: Handle explicit capture
bool captureByRef = Token::Match(lam.capture, "[ & ]");
bool captureByValue = Token::Match(lam.capture, "[ = ]");
auto captureVariable = [&](const Token* tok2, Lambda::Capture c, std::function<bool(const Token*)> pred) {
if (varids.count(tok->varId()) > 0)
return;
ErrorPath errorPath;
if (c == Lambda::Capture::ByReference) {
LifetimeStore{tok2, "Lambda captures variable by reference here.", ValueFlow::Value::LifetimeKind::Lambda} .byRef(
tok, tokenlist, errorLogger, settings, pred);
} else if (c == Lambda::Capture::ByValue) {
LifetimeStore{tok2, "Lambda captures variable by value here.", ValueFlow::Value::LifetimeKind::Lambda} .byVal(
tok, tokenlist, errorLogger, settings, pred);
}
};
// Handle explicit capture
for(const auto& p:lam.explicitCaptures) {
const Variable* var = p.first;
if (!var)
continue;
const Token* tok2 = p.second.first;
Lambda::Capture c = p.second.second;
captureVariable(tok2, c, [](const Token*) { return true; });
varids.insert(var->declarationId());
}
for (const Token * tok2 = lam.bodyTok; tok2 != lam.bodyTok->link(); tok2 = tok2->next()) {
ErrorPath errorPath;
if (captureByRef) {
LifetimeStore{tok2, "Lambda captures variable by reference here.", ValueFlow::Value::LifetimeKind::Lambda} .byRef(
tok, tokenlist, errorLogger, settings, isCapturingVariable);
} else if (captureByValue) {
LifetimeStore{tok2, "Lambda captures variable by value here.", ValueFlow::Value::LifetimeKind::Lambda} .byVal(
tok, tokenlist, errorLogger, settings, isCapturingVariable);
}
captureVariable(tok2, lam.implicitCapture, isImplicitCapturingVariable);
}
valueFlowForwardLifetime(tok, tokenlist, errorLogger, settings);
}
// address of
else if (tok->isUnaryOp("&")) {

View File

@ -1825,6 +1825,50 @@ private:
" return g([&]() { return x; });\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("auto f() {\n"
" int i = 0;\n"
" return [&i] {};\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning lambda that captures local variable 'i' that will be invalid when returning.\n", errout.str());
check("auto f() {\n"
" int i = 0;\n"
" return [i] {};\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("auto f() {\n"
" int i = 0;\n"
" return [=, &i] {};\n"
"}\n");
ASSERT_EQUALS("[test.cpp:3] -> [test.cpp:2] -> [test.cpp:3]: (error) Returning lambda that captures local variable 'i' that will be invalid when returning.\n", errout.str());
check("auto f() {\n"
" int i = 0;\n"
" int j = 0;\n"
" return [=, &i] { return j; };\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:2] -> [test.cpp:4]: (error) Returning lambda that captures local variable 'i' that will be invalid when returning.\n", errout.str());
check("auto f() {\n"
" int i = 0;\n"
" return [&, i] {};\n"
"}\n");
ASSERT_EQUALS("", errout.str());
check("auto f() {\n"
" int i = 0;\n"
" int j = 0;\n"
" return [&, i] { return j; };\n"
"}\n");
ASSERT_EQUALS("[test.cpp:4] -> [test.cpp:3] -> [test.cpp:4]: (error) Returning lambda that captures local variable 'j' that will be invalid when returning.\n", errout.str());
check("auto f(int& i) {\n"
" int j = 0;\n"
" return [=, &i] { return j; };\n"
"}\n");
ASSERT_EQUALS("", errout.str());
}
void danglingLifetimeContainer() {

View File

@ -7895,7 +7895,7 @@ private:
// `-(
// `-{
ASSERT_EQUALS("x{([( ai=", testAst("x([&a](int i){a=i;});"));
ASSERT_EQUALS("x{(a&[( ai=", testAst("x([&a](int i){a=i;});"));
ASSERT_EQUALS("{([(return 0return", testAst("return [](){ return 0; }();"));
// noexcept
@ -7903,40 +7903,40 @@ private:
// ->
ASSERT_EQUALS("{([(return 0return", testAst("return []() -> int { return 0; }();"));
ASSERT_EQUALS("{([(return 0return", testAst("return [something]() -> int { return 0; }();"));
ASSERT_EQUALS("{(something[(return 0return", testAst("return [something]() -> int { return 0; }();"));
ASSERT_EQUALS("{([cd,(return 0return", testAst("return [](int a, int b) -> int { return 0; }(c, d);"));
ASSERT_EQUALS("{([return", testAst("return []() -> decltype(0) {};"));
ASSERT_EQUALS("x{([=", testAst("x = [&]()->std::string const & {};"));
ASSERT_EQUALS("x{(&[=", testAst("x = [&]()->std::string const & {};"));
ASSERT_EQUALS("f{([=", testAst("f = []() -> foo* {};"));
ASSERT_EQUALS("f{([=", testAst("f = [](void) mutable -> foo* {};"));
ASSERT_EQUALS("f{([=", testAst("f = []() mutable {};"));
ASSERT_EQUALS("x{([= 0return", testAst("x = [](){return 0; };"));
ASSERT_EQUALS("ab{[(= cd=", testAst("a = b([&]{c=d;});"));
ASSERT_EQUALS("ab{&[(= cd=", testAst("a = b([&]{c=d;});"));
// 8628
ASSERT_EQUALS("f{([( switchx( 1case y++", testAst("f([](){switch(x){case 1:{++y;}}});"));
ASSERT_EQUALS("{([{return ab=",
ASSERT_EQUALS("{(=[{return ab=",
testAst("return {\n"
" [=]() {\n"
" a = b;\n"
" }\n"
"};\n"));
ASSERT_EQUALS("{[{return ab=",
ASSERT_EQUALS("{=[{return ab=",
testAst("return {\n"
" [=] {\n"
" a = b;\n"
" }\n"
"};\n"));
ASSERT_EQUALS("{([{return ab=",
ASSERT_EQUALS("{(=[{return ab=",
testAst("return {\n"
" [=]() -> int {\n"
" a=b;\n"
" }\n"
"}"));
ASSERT_EQUALS("{([{return ab=",
ASSERT_EQUALS("{(=[{return ab=",
testAst("return {\n"
" [=]() mutable -> int {\n"
" a=b;\n"
@ -7944,12 +7944,13 @@ private:
"}"));
// daca@home hang
ASSERT_EQUALS("a{([= 0return b{([= fori0=i10!=i++;;(",
ASSERT_EQUALS("a{(&[= 0return b{(=[= fori0=i10!=i++;;(",
testAst("a = [&]() -> std::pair<int, int> { return 0; };\n"
"b = [=]() { for (i = 0; i != 10; ++i); };"));
// #9662
ASSERT_EQUALS("b{[{ stdunique_ptr::0nullptrnullptr:?{", testAst("auto b{[] { std::unique_ptr<void *>{0 ? nullptr : nullptr}; }};"));
ASSERT_EQUALS("a( b{[=", testAst("void a() { [b = [] { ; }] {}; }"));
}