[cling] Make cling::utils::Lookup::Named look into using directives#15458
[cling] Make cling::utils::Lookup::Named look into using directives#15458devajithvs merged 1 commit intoroot-project:masterfrom
Conversation
devajithvs
changed the title
interpreter/cling/lib/Utils/AST.cpp
Outdated
| if (!res && primaryWithin->isNamespace()) | ||
| for (auto* UD : primaryWithin->using_directives()) | ||
| if (NamespaceDecl* NS = UD->getNominatedNamespace()) | ||
| res = S->LookupQualifiedName(R, NS); |
There was a problem hiding this comment.
Sema has an interface Sema::LookupName that follows the language rules. Can we use that one? We need to create some scope which describes the lookup position but I guess most of the time we care about the TUScope...
There was a problem hiding this comment.
Sema::LookupName is already used (since the inception of this function) on line 1492 for the TUScope. I suspect we did not know (do we know now?) how to create "some scope which describes the lookup position".
There was a problem hiding this comment.
I suspect we can push a scope with the namespace we want to look from, but we need to be careful to rebuild the scope nesting going from that namespace towards TU.
There was a problem hiding this comment.
Not sure if it's the best solution, I realized that clang doesn't look into namespaces unless the RedeclarationKind is set to Sema::NotForRedeclaration
There was a problem hiding this comment.
The intent of my comment was to avoid duplication of code that already exists in clang. The new version of that patch seems to address it.
|
Is there a corresponding roottest PR enabling/adding test for this? |
Test Results 14 files 14 suites 3d 10h 44m 0s ⏱️ Results for commit ca410ed. ♻️ This comment has been updated with latest results. |
Added a cling test for this. |
devajithvs
force-pushed
the
dev.fix_15407
branch
3 times, most recently
from
3267681 to
f938114
Compare
devajithvs
force-pushed
the
dev.fix_15407
branch
from
863ffc9 to
ca410ed
Compare
|
@devajithvs can you try enabling the test in |
|
Check running here: #16535 corresponding to root-project/roottest#1194 |
4 participants
This Pull request:
Changes or fixes:
Checklist:
This PR fixes #15407