Skip to content

fix #13967: Template simplifier: put Tokens into proper scope #7634

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ludviggunne
Copy link
Collaborator

No description provided.

@ludviggunne ludviggunne changed the title fix #13967: Template simplifier: put Tokens into propper scope fix #13967: Template simplifier: put Tokens into proper scope Jun 27, 2025
@@ -3209,6 +3210,18 @@ class TestSymbolDatabase : public TestFixture {
ASSERT_EQUALS(2U, fredAType->classDef->linenr());
}

void namespaces5() { // #13967
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is still about the TemplateSimplifier, but would a test there make sense?

@@ -173,6 +173,30 @@ void SymbolDatabase::createSymbolDatabaseFindAllScopes()

const bool doProgress = (mSettings.reportProgress != -1);

std::map<Scope *, std::set<std::string>> forwardDecls;

const std::function<Scope *(const Token *, Scope *)> findForwardDeclScope = [&](const Token *tok, Scope *startScope) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this be a static function?

Copy link
Owner

@danmar danmar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the ticket/PR should be renamed to indicate that the symboldatabase is fixed and that we don't touch the template simplifier.

the tokens are still written in global scope right?

const Variable *x = db->getVariableFromVarId(2U);
ASSERT_EQUALS("x", x->name());
const Scope *scope = x->scope();
ASSERT_EQUALS("test", scope->nestedIn->className);
Copy link
Owner

@danmar danmar Jun 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this test will not work well if scope->nestedIn is nullptr I want an extra ASSERT to ensure a clear error report is written..

" const Test<64> test;\n"
"}\n");
const Variable *x = db->getVariableFromVarId(2U);
ASSERT_EQUALS("x", x->name());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm I feel this test is a bit weird. Variable ids are just unique numbers. If the variable "x" would get varid 1002 in the future nothing is wrong about that but this test will fail. But on the other hand we have lots of similar tests already in testsymboldatabase so you just followed the common pattern!
So we can keep it..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants