[cling] Make cling::utils::Lookup::Named look into using directives #15458
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? |
Added a cling test for this. |
devajithvs
force-pushed
the
dev.fix_15407
branch
3 times, most recently
from
3267681
to
f938114
Compare
| @@ -6,6 +6,7 @@ | |||
| // LICENSE.TXT for details. | |||
| //------------------------------------------------------------------------------ | |||
|
|
|||
| // clang-format off | |||
There was a problem hiding this comment.
Can we put a .clang-format file in the test folder instructing clang-format to ignore that folder?
This Pull request:
Changes or fixes:
Checklist:
This PR fixes #15407