Skip to content

[clang] Heuristic resolution for explicit object parameter #155143

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Aug 27, 2025
Merged

[clang] Heuristic resolution for explicit object parameter #155143

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c1cdb39
[clang] Heuristic resolution for explicit object parameter
MythreyaK Aug 24, 2025
0d48289
code review
MythreyaK Aug 24, 2025
cd3601a
remove cxxdecl check
MythreyaK Aug 24, 2025
ea15161
code review
MythreyaK Aug 24, 2025
c2b8ac4
add null-check
MythreyaK Aug 24, 2025
bf73e27
code review
MythreyaK Aug 26, 2025
b48fa0e
code review + misc cleanup
MythreyaK Aug 27, 2025
466efd8
fix error
MythreyaK Aug 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion clang-tools-extra/clangd/unittests/RenameTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@
#include "clang/Tooling/Core/Replacement.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/Support/MemoryBuffer.h"
#include <algorithm>
#include "gmock/gmock.h"
#include "gtest/gtest.h"

#include <algorithm>

namespace clang {
namespace clangd {
namespace {
Expand Down Expand Up @@ -2468,6 +2469,46 @@ TEST(CrossFileRenameTests, adjustmentCost) {
}
}

TEST(RenameTest, RenameWithExplicitObjectPararameter) {
Copy link
Collaborator

@HighCommander4 HighCommander4 Aug 25, 2025

Choose a reason for hiding this comment

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

Instead of adding a new top-level test case (which involves repeating some boilerplate), could you add an entry to the array in RenameTest.WithinFileRename instead (and adjust the flags of that test to use -std=c++23 as necessary)?

Annotations Test = {R"cpp(
struct Foo {
int [[memb^er]] {};
auto&& getter1(this auto&& self) {
auto local = [&] {
return self.[[memb^er]];
}();
return local + self.[[memb^er]];
}
auto&& getter2(this Foo&& self) {
return self.[[memb^er]];
}
int normal() {
return this->[[mem^ber]] + [[memb^er]];
}
};
)cpp"};

auto TU = TestTU::withCode(Test.code());
TU.ExtraArgs.push_back("-std=c++23");
auto AST = TU.build();

llvm::StringRef NewName = "m_member";
auto Index = TU.index();

for (const auto &RenamePos : Test.points()) {
auto RenameResult = rename({RenamePos, NewName, AST, testPath(TU.Filename),
getVFSFromAST(AST), Index.get()});

ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
auto Res = RenameResult.get();

ASSERT_TRUE(bool(RenameResult)) << RenameResult.takeError();
ASSERT_EQ(1u, RenameResult->GlobalChanges.size());
EXPECT_EQ(applyEdits(std::move(RenameResult->GlobalChanges)).front().second,
expectedResult(Test, NewName));
}
}

} // namespace
} // namespace clangd
} // namespace clang
25 changes: 25 additions & 0 deletions clang/lib/Sema/HeuristicResolver.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "clang/AST/TemplateBase.h"
#include "clang/AST/Type.h"
#include "llvm/ADT/identity.h"
#include <optional>

namespace clang {

Expand Down Expand Up @@ -301,10 +302,34 @@ std::vector<const NamedDecl *> HeuristicResolverImpl::resolveMemberExpr(
return {};
}

// check if member expr is in the context of an explicit object method
// If so, it's safe to assume the templated arg is of type of the record
Copy link
Contributor

@cor3ntin cor3ntin Aug 24, 2025

Choose a reason for hiding this comment

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

Consider things like

struct S {
   template <typename T>
   void f(this Foo<T>);
};

It might be prudent to
1/ call simplifyType first
2/ only apply the heuristic to undeduced auto and TemplateTypeParmType

Copy link
Contributor Author

@MythreyaK MythreyaK Aug 24, 2025

Choose a reason for hiding this comment

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

I tried to address this, but not sure if this is what you meant (I'm new to clang and still finding my way 😄). Please let me know what you think!

   template <typename T>
   void f(this Foo<T>);

Can a this parameter be something other than a record not related to S (Foo<T>, in this example)?

const auto ExplicitMemberHeuristic = [&](const Expr *Base) -> QualType {
if (auto *DeclRef = dyn_cast_if_present<DeclRefExpr>(Base)) {
auto *PrDecl = dyn_cast_if_present<ParmVarDecl>(DeclRef->getDecl());

Copy link
Contributor

@cor3ntin cor3ntin Aug 24, 2025

Choose a reason for hiding this comment

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

When is that null?

Copy link
Contributor Author

@MythreyaK MythreyaK Aug 24, 2025

Choose a reason for hiding this comment

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

PrDecl is null here, for example.

DeclRefExpr 0x2e63d268 'auto' lvalue Var 0x2e63d150 'b' 'auto'

Copy link
Collaborator

@HighCommander4 HighCommander4 Aug 27, 2025

Choose a reason for hiding this comment

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

I think the question is rather, when is DeclRef->getDecl() null, such that you need to use dyn_cast_if_present rather than plain dyn_cast.

Looking at other usages of DeclRefExpr::getDecl(), it seems callers assume it's never null.

Copy link
Contributor Author

@MythreyaK MythreyaK Aug 27, 2025

Choose a reason for hiding this comment

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

Updated to use dyn_cast instead 👍

if (PrDecl && PrDecl->isExplicitObjectParameter()) {
// get the parent, a cxxrecord
return Ctx.getTypeDeclType(
dyn_cast<TypeDecl>(PrDecl->getDeclContext()->getParent()));
}
}

return {};
};

// Try resolving the member inside the expression's base type.
Expr *Base = ME->isImplicitAccess() ? nullptr : ME->getBase();
QualType BaseType = ME->getBaseType();
BaseType = simplifyType(BaseType, Base, ME->isArrow());

if (!BaseType.isNull() &&
Copy link
Collaborator

@HighCommander4 HighCommander4 Aug 25, 2025

Choose a reason for hiding this comment

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

Can we move this into simplifyType itself? Specifically, inside SimplifyOneStep, what are BaseType and Base here are available as T.Type and T.Expr respectively.

That would make this heuristic a bit more general. (For example, in combination with this patch which adds a simplifyType call site in SemaCodeComplete, it gets code completion to work after self.^ in such a function.)

(BaseType->isUndeducedAutoType() || BaseType->isTemplateTypeParmType())) {
if (auto Type = ExplicitMemberHeuristic(Base); !Type.isNull()) {
BaseType = Type;
}
}

return resolveDependentMember(BaseType, ME->getMember(), NoFilter);
}

Expand Down
19 changes: 18 additions & 1 deletion clang/unittests/Sema/HeuristicResolverTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ template <typename InputNode, typename ParamT, typename InputMatcher,
typename... OutputMatchers>
void expectResolution(llvm::StringRef Code, ResolveFnT<ParamT> ResolveFn,
const InputMatcher &IM, const OutputMatchers &...OMS) {
auto TU = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++20"});
auto TU = tooling::buildASTFromCodeWithArgs(Code, {"-std=c++23"});
auto &Ctx = TU->getASTContext();
auto InputMatches = match(IM, Ctx);
ASSERT_EQ(1u, InputMatches.size());
Expand Down Expand Up @@ -449,6 +449,23 @@ TEST(HeuristicResolver, MemberExpr_DefaultTemplateArgument_Recursive) {
cxxMethodDecl(hasName("foo")).bind("output"));
}

TEST(HeuristicResolver, MemberExpr_ExplicitObjectParameter) {
std::string Code = R"cpp(
struct Foo {
int m_int;

int bar(this auto&& self) {
return self.m_int;
}
};
)cpp";
// Test resolution of "m_int" in "self.m_int()".
expectResolution(
Code, &HeuristicResolver::resolveMemberExpr,
cxxDependentScopeMemberExpr(hasMemberName("m_int")).bind("input"),
fieldDecl(hasName("m_int")).bind("output"));
}

TEST(HeuristicResolver, DeclRefExpr_StaticMethod) {
std::string Code = R"cpp(
template <typename T>
Expand Down
Loading