-
Notifications
You must be signed in to change notification settings - Fork 60
Fix: Resolve inherited members through implicit UsingShadowDecls #1038
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?
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -1193,6 +1193,39 @@ clang::NamedDecl* LookupUnqualified(clang::Sema& S, | |||||
| return (clang::NamedDecl*)-1; | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
| static clang::UsingShadowDecl *CreateInheritedUsingShadow( | ||||||
| clang::CXXRecordDecl *Record, clang::NamedDecl *Target) { | ||||||
| if (!Record || !Target) | ||||||
| return nullptr; | ||||||
|
|
||||||
| if (auto *Shadow = llvm::dyn_cast<clang::UsingShadowDecl>(Target)) | ||||||
| Target = Shadow->getTargetDecl(); | ||||||
|
|
||||||
| if (!Target) | ||||||
| return nullptr; | ||||||
|
|
||||||
| if (Target->getDeclContext() == Record) | ||||||
| return llvm::dyn_cast<clang::UsingShadowDecl>(Target); | ||||||
|
|
||||||
| clang::ASTContext &C = Record->getASTContext(); | ||||||
| clang::DeclarationNameInfo NameInfo(Target->getDeclName(), | ||||||
| clang::SourceLocation()); | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: use nullptr [modernize-use-nullptr]
Suggested change
|
||||||
| auto *Using = clang::UsingDecl::Create(C, Record, clang::SourceLocation(), | ||||||
| clang::NestedNameSpecifierLoc(), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: use nullptr [modernize-use-nullptr]
Suggested change
|
||||||
| NameInfo, false); | ||||||
| Using->setImplicit(true); | ||||||
|
|
||||||
| auto *Shadow = clang::UsingShadowDecl::Create( | ||||||
| C, Record, clang::SourceLocation(), Target->getDeclName(), Using, | ||||||
| Target); | ||||||
| Using->addShadowDecl(Shadow); | ||||||
| Shadow->setAccess(Target->getAccess()); | ||||||
| Record->addDecl(Shadow); | ||||||
|
|
||||||
| return Shadow; | ||||||
| } | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no header providing "CppInternal::utils::Lookup::Named" is directly included [misc-include-cleaner] lib/CppInterOp/CppInterOp.cpp:10: - #include "Unwrap.h"
+ #include "CppInterOpInterpreter.h"
+ #include "Unwrap.h" |
||||||
|
|
||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast] hin);
^
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr] hin);
^ |
||||||
| // Cheap probe: does any namespace from `DC` up to TU carry at least | ||||||
| // one using-directive? Gates the synthetic-DRef-chain build below so | ||||||
| // the common case (no using-directives anywhere on the path) doesn't | ||||||
|
|
@@ -1230,6 +1263,40 @@ DeclRef GetNamed(const std::string& name, ConstDeclRef parent /*= nullptr*/) { | |||||
| if (ND && ND != (clang::NamedDecl*)-1) | ||||||
| return INTEROP_RETURN(ND->getCanonicalDecl()); | ||||||
|
|
||||||
| // Qualified lookup can miss inherited members in class scope. Try the | ||||||
| // record's own lookup, which includes direct members, and then fall back | ||||||
| // to unqualified lookup inside the record to honor base-class member | ||||||
| // visibility semantics. | ||||||
| if (Within) { | ||||||
| if (auto* RD = llvm::dyn_cast<clang::CXXRecordDecl>(Within)) { | ||||||
| clang::DeclarationName DName = &getSema().Context.Idents.get(name); | ||||||
| auto Decls = RD->lookup(DName); | ||||||
| clang::NamedDecl* FoundND = nullptr; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast] hin);
^
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr] hin);
^ |
||||||
| for (auto* D : Decls) { | ||||||
| if (auto* Named = llvm::dyn_cast<clang::NamedDecl>(D)) { | ||||||
| if (!FoundND) | ||||||
| FoundND = Named; | ||||||
| else | ||||||
| return INTEROP_RETURN(nullptr); | ||||||
| } | ||||||
| } | ||||||
| if (FoundND) | ||||||
| return INTEROP_RETURN((FoundND->getCanonicalDecl())); | ||||||
|
|
||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast] DRef.
^ |
||||||
| // Qualified lookup may still miss inherited class members in some | ||||||
| // record contexts. Use unqualified lookup from a synthesized point | ||||||
| // inside the class to traverse base classes and find the member. | ||||||
| auto* ND2 = LookupUnqualified(getSema(), DName, Within); | ||||||
| if (ND2 && ND2 != (clang::NamedDecl*)-1) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast] hin);
^
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr] hin);
^ |
||||||
| if (auto* Shadow = CreateInheritedUsingShadow(RD, ND2)) | ||||||
| return INTEROP_RETURN((Shadow->getCanonicalDecl())); | ||||||
| return INTEROP_RETURN((ND2->getCanonicalDecl())); | ||||||
| } | ||||||
| if (ND2 == (clang::NamedDecl*)-1) | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: do not use C-style cast to convert between unrelated types [cppcoreguidelines-pro-type-cstyle-cast] }
^
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: integer to pointer cast pessimizes optimization opportunities [performance-no-int-to-ptr] }
^ |
||||||
| return INTEROP_RETURN(nullptr); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| // Slow path: only when qualified lookup missed AND `Within` is a | ||||||
| // namespace whose enclosing chain carries at least one using-directive | ||||||
| // (the only reason qualified-vs-unqualified disagree at namespace | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -702,6 +702,26 @@ TYPED_TEST(CPPINTEROP_TEST_MODE, ScopeReflection_GetNamed) { | |||||||||
| EXPECT_EQ(Cpp::GetQualifiedName(std_ns), "std"); | ||||||||||
| EXPECT_EQ(Cpp::GetQualifiedName(std_string_class), "std::string"); | ||||||||||
| EXPECT_EQ(Cpp::GetQualifiedName(std_string_npos_var), "std::basic_string<char>::npos"); | ||||||||||
|
|
||||||||||
| Interp->declare(R"( | ||||||||||
| struct S { | ||||||||||
| typedef int Val; | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| struct S1 : public S { | ||||||||||
| /* empty */ | ||||||||||
| }; | ||||||||||
| )"); | ||||||||||
|
|
||||||||||
| Cpp::TCppScope_t strt_S = Cpp::GetNamed("S", nullptr); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no type named 'TCppScope_t' in namespace 'Cpp' [clang-diagnostic-error] Cpp::TCppScope_t strt_S = Cpp::GetNamed("S", nullptr);
^ |
||||||||||
| Cpp::TCppScope_t strt_S_Val = Cpp::GetNamed("Val", strt_S); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no type named 'TCppScope_t' in namespace 'Cpp' [clang-diagnostic-error] Cpp::TCppScope_t strt_S_Val = Cpp::GetNamed("Val", strt_S);
^ |
||||||||||
| Cpp::TCppScope_t strt_S1 = Cpp::GetNamed("S1", nullptr); | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: expected ';' after expression [clang-diagnostic-error]
Suggested change
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: no member named 'TCppScope_t' in namespace 'Cpp' [clang-diagnostic-error] Cpp::TCppScope_t strt_S1 = Cpp::GetNamed("S1", nullptr);
^
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. warning: use of undeclared identifier 'strt_S1'; did you mean 'strt_S'? [clang-diagnostic-error]
Suggested change
Additional contextunittests/CppInterOp/ScopeReflectionTest.cpp:715: 'strt_S' declared here Cpp::TCppScope_t strt_S = Cpp::GetNamed("S", nullptr);
^ |
||||||||||
| Cpp::TCppScope_t strt_S1_Val = Cpp::GetNamed("Val", strt_S1); | ||||||||||
|
|
||||||||||
| EXPECT_EQ(Cpp::GetQualifiedName(strt_S), "S"); | ||||||||||
| EXPECT_EQ(Cpp::GetQualifiedName(strt_S_Val), "S::Val"); | ||||||||||
| EXPECT_EQ(Cpp::GetQualifiedName(strt_S1), "S1"); | ||||||||||
| EXPECT_EQ(Cpp::GetQualifiedName(strt_S1_Val), "S1::Val"); | ||||||||||
| } | ||||||||||
|
|
||||||||||
| TYPED_TEST(CPPINTEROP_TEST_MODE, ScopeReflection_GetNamedWithUsing) { | ||||||||||
|
|
||||||||||
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.
warning: 'CreateInheritedUsingShadow' is a static definition in anonymous namespace; static is redundant here [readability-static-definition-in-anonymous-namespace]