-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
base: main
Are you sure you want to change the base?
Conversation
@@ -3209,6 +3210,18 @@ class TestSymbolDatabase : public TestFixture { | |||
ASSERT_EQUALS(2U, fredAType->classDef->linenr()); | |||
} | |||
|
|||
void namespaces5() { // #13967 |
There was a problem hiding this comment.
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) { |
There was a problem hiding this comment.
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?
There was a problem hiding this 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); |
There was a problem hiding this comment.
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()); |
There was a problem hiding this comment.
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..
No description provided.