Incorrectly handled foreach style loops#591
Incorrectly handled foreach style loops#591zsofiaschell wants to merge 8 commits intoEricsson:masterfrom
Conversation
Sync forked repo
mcserep
added
Kind: Bug ⚠️
Plugin: C++
mcserep
left a comment
mcserep
left a comment
There was a problem hiding this comment.
Regular variables seems to be handled correctly now, but iterating over function and method call results not.
I have used the following sample code for testing:
https://github.com/mcserep/cc-tests/blob/master/foreach/main.cpp
|
|
||
| clang::Stmt* stmt = llvm::dyn_cast<clang::Stmt>(forRangeStmt->getRangeInit()); | ||
|
|
||
| clang::CallExpr* callExpr = llvm::dyn_cast<clang::CallExpr>(stmt); |
There was a problem hiding this comment.
When we have a function or method call in the range.based for loop, the forRangeStmt->getRangeInit() will be a ExprWithCleanups statement, and one of its descendant in the AST will be the CallExpr or CXXMemberCallExpr.
Eg. for a function call:
And for a method call:
There was a problem hiding this comment.
@whisperity: is there an easy way to request the first descendant of a node which matches a certain type? E.g. to get the first CallExpr descendant of the ExprWithCleanups node in the AST?
There was a problem hiding this comment.
Unfortunately, I think no. Every node is implemented with different fields and accessor methods. The whole RecursiveASTVisitor is using lots of generated code in the background to facilitate the traversals. We could try using the AST Matchers library (that Clang-Tidy also uses...) that has a generic has() matcher which can bind a child node.
| astNode->astValue = getSourceText( | ||
| _clangSrcMgr, | ||
| forRangeStmt->getRangeStmt()->getBeginLoc(), | ||
| forRangeStmt->getRangeStmt()->getEndLoc(), | ||
| true); | ||
|
|
||
| astNode->location = getFileLoc(forRangeStmt->getBeginLoc(), forRangeStmt->getEndLoc()); | ||
|
|
||
| const clang::VarDecl* vd = llvm::dyn_cast<clang::VarDecl>(forRangeStmt->getRangeStmt()->getSingleDecl()); | ||
| astNode->entityHash = util::fnvHash(getUSR(vd)); | ||
|
|
||
| astNode->symbolType = model::CppAstNode::SymbolType::Variable; | ||
|
|
||
| astNode->astType = model::CppAstNode::AstType::Read; | ||
|
|
||
| astNode->id = model::createIdentifier(*astNode); | ||
|
|
||
| _astNodes.push_back(astNode); |
There was a problem hiding this comment.
Since you also call VisitDeclRefExpr, this seems to be redundant.
mcserep
added
the
Status: At risk 🚨
3 participants
Fix #509
TraverseCXXForRangeStmt and VisitCXXForRangeStmt added to handle foreach style loops correctly.