Skip to content

Commit 27afce0

Browse files
committed
Fix #15143 Improve performance of setVarIdPass2
https://trac.cppcheck.net/ticket/15143 Tokenizer::setVarIdPass2() tried every out-of-class member definition for every class. matchMemberName() can only succeed when the innermost scope name of the class matches a name in the member's scope, in one of its "using namespace" or in its qualification. The members are now indexed by those names, in their original order, so each class only tries its candidates. The index and matchMemberName() walk the qualification with the same skipQualification(). On the same project total CPU time drops further from 61.6 to 52.9 minutes, with identical output for every translation unit.
1 parent 4fec879 commit 27afce0

2 files changed

Lines changed: 109 additions & 12 deletions

File tree

‎lib/tokenize.cpp‎

Lines changed: 65 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5220,6 +5220,20 @@ static std::string getScopeName(const std::list<ScopeInfo2> &scopeInfo)
52205220
return ret;
52215221
}
52225222

5223+
// If tok is the name in a qualification "name ::" or "name < ... > ::", return the token after the "::"
5224+
static Token * skipQualification(Token *tok)
5225+
{
5226+
if (!Token::Match(tok, "%name% ::|<"))
5227+
return nullptr;
5228+
if (tok->strAt(1) == "<") {
5229+
tok = tok->next()->findClosingBracket();
5230+
if (!Token::simpleMatch(tok, "> ::"))
5231+
return nullptr;
5232+
}
5233+
return tok->tokAt(2);
5234+
}
5235+
5236+
// indexMembersByScopeName() relies on how this matches the scope names: keep the two in sync
52235237
static Token * matchMemberName(const std::list<std::string> &scope, const Token *nsToken, Token *memberToken, const std::list<ScopeInfo2> &scopeInfo)
52245238
{
52255239
auto scopeIt = scopeInfo.cbegin();
@@ -5252,16 +5266,11 @@ static Token * matchMemberName(const std::list<std::string> &scope, const Token
52525266

52535267
// Parse member tokens..
52545268
while (scopeIt != scopeInfo.end()) {
5255-
if (!Token::Match(memberToken, "%name% ::|<"))
5269+
if (!Token::Match(memberToken, "%name%") || memberToken->str() != scopeIt->name)
52565270
return nullptr;
5257-
if (memberToken->str() != scopeIt->name)
5271+
memberToken = skipQualification(memberToken);
5272+
if (!memberToken)
52585273
return nullptr;
5259-
if (memberToken->strAt(1) == "<") {
5260-
memberToken = memberToken->next()->findClosingBracket();
5261-
if (!Token::simpleMatch(memberToken, "> ::"))
5262-
return nullptr;
5263-
}
5264-
memberToken = memberToken->tokAt(2);
52655274
++scopeIt;
52665275
}
52675276

@@ -5288,6 +5297,47 @@ static Token * matchMemberName(const Member &member, const std::list<ScopeInfo2>
52885297
return nullptr;
52895298
}
52905299

5300+
using MembersByScopeName = std::unordered_map<std::string, std::vector<const Member *>>;
5301+
5302+
// matchMemberName() can only match when the last scope name is matched by a name in the
5303+
// member's scope, in one of its "using namespace" or in its qualification. Index the members
5304+
// by these names, keeping their order, so that each scope only tries its candidates.
5305+
static MembersByScopeName indexMembersByScopeName(const std::list<Member> &members)
5306+
{
5307+
MembersByScopeName index;
5308+
for (const Member &member : members) {
5309+
// members are visited in order, so a member that is already a candidate is the last one
5310+
const auto addCandidate = [&](const std::string &name) {
5311+
std::vector<const Member *> &candidates = index[name];
5312+
if (candidates.empty() || candidates.back() != &member)
5313+
candidates.push_back(&member);
5314+
};
5315+
for (const std::string &name : member.scope)
5316+
addCandidate(name);
5317+
for (const Token *ns : member.usingnamespaces) {
5318+
for (; Token::Match(ns, "%name% ::"); ns = ns->tokAt(2))
5319+
addCandidate(ns->str());
5320+
if (Token::Match(ns, "%name%"))
5321+
addCandidate(ns->str());
5322+
}
5323+
Token *tok = member.tok;
5324+
while (Token *next = skipQualification(tok)) {
5325+
addCandidate(tok->str());
5326+
tok = next;
5327+
}
5328+
}
5329+
return index;
5330+
}
5331+
5332+
static const std::vector<const Member *> &membersForScope(const MembersByScopeName &index, const std::list<ScopeInfo2> &scopeInfo)
5333+
{
5334+
static const std::vector<const Member *> none;
5335+
if (scopeInfo.empty())
5336+
return none;
5337+
const auto it = index.find(scopeInfo.back().name);
5338+
return it == index.end() ? none : it->second;
5339+
}
5340+
52915341
static Token * matchMemberVarName(const Member &var, const std::list<ScopeInfo2> &scopeInfo)
52925342
{
52935343
Token *tok = matchMemberName(var, scopeInfo);
@@ -5399,6 +5449,9 @@ void Tokenizer::setVarIdPass2()
53995449
}
54005450
}
54015451

5452+
const MembersByScopeName memberFunctionsByScopeName = indexMembersByScopeName(allMemberFunctions);
5453+
const MembersByScopeName memberVarsByScopeName = indexMembersByScopeName(allMemberVars);
5454+
54025455
std::list<ScopeInfo2> scopeInfo;
54035456

54045457
// class members..
@@ -5515,8 +5568,8 @@ void Tokenizer::setVarIdPass2()
55155568
continue;
55165569

55175570
// Member variables
5518-
for (const Member &var : allMemberVars) {
5519-
Token *tok2 = matchMemberVarName(var, scopeInfo);
5571+
for (const Member *var : membersForScope(memberVarsByScopeName, scopeInfo)) {
5572+
Token *tok2 = matchMemberVarName(*var, scopeInfo);
55205573
if (!tok2)
55215574
continue;
55225575
if (tok2->varId() == 0)
@@ -5527,8 +5580,8 @@ void Tokenizer::setVarIdPass2()
55275580
continue;
55285581

55295582
// Set variable ids in member functions for this class..
5530-
for (const Member &func : allMemberFunctions) {
5531-
Token *tok2 = matchMemberFunctionName(func, scopeInfo);
5583+
for (const Member *func : membersForScope(memberFunctionsByScopeName, scopeInfo)) {
5584+
Token *tok2 = matchMemberFunctionName(*func, scopeInfo);
55325585
if (!tok2)
55335586
continue;
55345587

‎test/testvarid.cpp‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,8 @@ class TestVarID : public TestFixture {
240240
TEST_CASE(usingNamespace1);
241241
TEST_CASE(usingNamespace2);
242242
TEST_CASE(usingNamespace3);
243+
TEST_CASE(usingNamespace4);
244+
TEST_CASE(usingNamespace5);
243245

244246
TEST_CASE(setVarIdStructMembers1);
245247

@@ -4410,6 +4412,48 @@ class TestVarID : public TestFixture {
44104412
ASSERT_EQUALS(expected, tokenize(code));
44114413
}
44124414

4415+
void usingNamespace4() {
4416+
// the innermost scope name is matched by the qualification of the member, after a template argument list
4417+
const char code[] = "namespace A {\n"
4418+
" namespace B {\n"
4419+
" template<class T> struct C {\n"
4420+
" struct D { int x; void f(); };\n"
4421+
" };\n"
4422+
" }\n"
4423+
"}\n"
4424+
"using namespace A;\n"
4425+
"template<class T> void B::C<T>::D::f() { x = 0; }\n";
4426+
4427+
const char expected[] = "1: namespace A {\n"
4428+
"2: namespace B {\n"
4429+
"3: template < class T > struct C {\n"
4430+
"4: struct D { int x@1 ; void f ( ) ; } ;\n"
4431+
"5: } ;\n"
4432+
"6: }\n"
4433+
"7: }\n"
4434+
"8: using namespace A ;\n"
4435+
"9: template < class T > void B :: C < T > :: D :: f ( ) { x@1 = 0 ; }\n";
4436+
4437+
ASSERT_EQUALS(expected, tokenize(code));
4438+
}
4439+
4440+
void usingNamespace5() {
4441+
// the innermost scope name is matched by the "using namespace"
4442+
const char code[] = "namespace A {\n"
4443+
" namespace B { int x; }\n"
4444+
"}\n"
4445+
"using namespace A::B;\n"
4446+
"void f() { x = 0; }\n";
4447+
4448+
const char expected[] = "1: namespace A {\n"
4449+
"2: namespace B { int x@1 ; }\n"
4450+
"3: }\n"
4451+
"4: using namespace A :: B ;\n"
4452+
"5: void f ( ) { x@1 = 0 ; }\n";
4453+
4454+
ASSERT_EQUALS(expected, tokenize(code));
4455+
}
4456+
44134457
void setVarIdStructMembers1() {
44144458
const char code[] = "void f(Foo foo)\n"
44154459
"{\n"

0 commit comments

Comments
 (0)