Skip to content

Fix: Resolve inherited members through implicit UsingShadowDecls#1038

Draft
Krishna-13-cyber wants to merge 3 commits into
compiler-research:mainfrom
Krishna-13-cyber:Shadow-classdecl
Draft

Fix: Resolve inherited members through implicit UsingShadowDecls#1038
Krishna-13-cyber wants to merge 3 commits into
compiler-research:mainfrom
Krishna-13-cyber:Shadow-classdecl

Conversation

@Krishna-13-cyber

Copy link
Copy Markdown
Contributor

Description

Please include a summary of changes, motivation and context for this PR.
Member lookup considered declarations could miss members inherited from base classes in some record lookup contexts.

The change adds a fallback to unqualified lookup when direct record lookup fails. This allows lookup to traverse base classes and find inherited members. When an inherited member is found, an implicit UsingShadowDecl is created in the derived class to represent that inherited member as if it were introduced into the derived class scope
UsingShadowDecl mechanism should handle inherited typedefs as well.

Fixes # (issue)
#93 fixes this issue.

Type of change

Please tick all options which are relevant.

  • [ X ] Bug fix
  • New feature
  • Requires documentation updates

Testing

Please describe the test(s) that you added and ran to verify your changes.
https://github.com/compiler-research/CppInterOp/pull/123/changes
this is the reproducible test, which gets passed after this patch.

Checklist

  • I have read the contribution guide recently

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 19. Check the log or trigger a new build to see more.

}


static clang::UsingShadowDecl *CreateInheritedUsingShadow(

Copy link
Copy Markdown
Contributor

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]

Suggested change
static clang::UsingShadowDecl *CreateInheritedUsingShadow(
;

Comment thread lib/CppInterOp/CppInterOp.cpp Outdated
}
}
if (FoundND)
return INTEROP_RETURN((TCppScope_t)(FoundND->getCanonicalDecl()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: use of undeclared identifier 'TCppScope_t' [clang-diagnostic-error]

ndND)
                                     ^

// 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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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);
                              ^

// 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) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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);
                              ^

Comment thread lib/CppInterOp/CppInterOp.cpp Outdated
auto* ND2 = LookupUnqualified(getSema(), DName, Within);
if (ND2 && ND2 != (clang::NamedDecl*)-1) {
if (auto* Shadow = CreateInheritedUsingShadow(RD, ND2))
return INTEROP_RETURN((TCppScope_t)(Shadow->getCanonicalDecl()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: use of undeclared identifier 'TCppScope_t' [clang-diagnostic-error]

ND2))
                                       ^

Comment thread lib/CppInterOp/CppInterOp.cpp Outdated
if (ND2 && ND2 != (clang::NamedDecl*)-1) {
if (auto* Shadow = CreateInheritedUsingShadow(RD, ND2))
return INTEROP_RETURN((TCppScope_t)(Shadow->getCanonicalDecl()));
return INTEROP_RETURN((TCppScope_t)(ND2->getCanonicalDecl()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: use of undeclared identifier 'TCppScope_t' [clang-diagnostic-error]

()));
                                     ^

return INTEROP_RETURN((TCppScope_t)(Shadow->getCanonicalDecl()));
return INTEROP_RETURN((TCppScope_t)(ND2->getCanonicalDecl()));
}
if (ND2 == (clang::NamedDecl*)-1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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]

    }
                       ^

return INTEROP_RETURN((TCppScope_t)(Shadow->getCanonicalDecl()));
return INTEROP_RETURN((TCppScope_t)(ND2->getCanonicalDecl()));
}
if (ND2 == (clang::NamedDecl*)-1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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]

    }
                       ^

};
)");

Cpp::TCppScope_t strt_S = Cpp::GetNamed("S", nullptr);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 = Cpp::GetNamed("S", nullptr);
Cpp::TCppScope_t strt_S_Val = Cpp::GetNamed("Val", strt_S);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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);
     ^

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

There were too many comments to post at once. Showing the first 10 out of 16. Check the log or trigger a new build to see more.


clang::ASTContext &C = Record->getASTContext();
clang::DeclarationNameInfo NameInfo(Target->getDeclName(),
clang::SourceLocation());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: use nullptr [modernize-use-nullptr]

Suggested change
clang::SourceLocation());
= nullptr;

Record->addDecl(Shadow);

return Shadow;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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"


return Shadow;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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);
                        ^


return Shadow;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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* RD = llvm::dyn_cast<clang::CXXRecordDecl>(Within)) {
clang::DeclarationName DName = &getSema().Context.Idents.get(name);
auto Decls = RD->lookup(DName);
clang::NamedDecl* FoundND = nullptr;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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);
                        ^

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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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 (FoundND)
return INTEROP_RETURN((FoundND->getCanonicalDecl()));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.
                  ^


Cpp::TCppScope_t strt_S = Cpp::GetNamed("S", nullptr);
Cpp::TCppScope_t strt_S_Val = Cpp::GetNamed("Val", strt_S);
Cpp::TCppScope_t strt_S1 = Cpp::GetNamed("S1", nullptr);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

warning: expected ';' after expression [clang-diagnostic-error]

Suggested change
Cpp::TCppScope_t strt_S1 = Cpp::GetNamed("S1", nullptr);
Cpp::TCppScope_t; strt_S1 = Cpp::GetNamed("S1", nullptr);


Cpp::TCppScope_t strt_S = Cpp::GetNamed("S", nullptr);
Cpp::TCppScope_t strt_S_Val = Cpp::GetNamed("Val", strt_S);
Cpp::TCppScope_t strt_S1 = Cpp::GetNamed("S1", nullptr);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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);
     ^


Cpp::TCppScope_t strt_S = Cpp::GetNamed("S", nullptr);
Cpp::TCppScope_t strt_S_Val = Cpp::GetNamed("Val", strt_S);
Cpp::TCppScope_t strt_S1 = Cpp::GetNamed("S1", nullptr);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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
Cpp::TCppScope_t strt_S1 = Cpp::GetNamed("S1", nullptr);
Cpp::TCppScope_t strt_S = Cpp::GetNamed("S1", nullptr);
Additional context

unittests/CppInterOp/ScopeReflectionTest.cpp:715: 'strt_S' declared here

Cpp::TCppScope_t strt_S = Cpp::GetNamed("S", nullptr);
                 ^

@codecov

codecov Bot commented Jun 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.28571% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.44%. Comparing base (a1c1fc2) to head (3f3854e).
⚠️ Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
lib/CppInterOp/CppInterOp.cpp 94.28% 2 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1038      +/-   ##
==========================================
+ Coverage   86.23%   86.44%   +0.20%     
==========================================
  Files          21       23       +2     
  Lines        5369     5658     +289     
==========================================
+ Hits         4630     4891     +261     
- Misses        739      767      +28     
Files with missing lines Coverage Δ
lib/CppInterOp/CppInterOp.cpp 89.31% <94.28%> (-0.01%) ⬇️

... and 4 files with indirect coverage changes

Files with missing lines Coverage Δ
lib/CppInterOp/CppInterOp.cpp 89.31% <94.28%> (-0.01%) ⬇️

... and 4 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

1 participant