Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
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

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 [[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
26 changes: 26 additions & 0 deletions clang/lib/Sema/HeuristicResolver.cpp
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,9 +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

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

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) -> std::optional<QualType> {
if (auto *DeclRef = dyn_cast_if_present<DeclRefExpr>(Base)) {
auto *PrDecl = dyn_cast_if_present<ParmVarDecl>(DeclRef->getDecl());

Copy link
Contributor

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

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

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

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()) {
auto CxxRecord = dyn_cast_if_present<CXXRecordDecl>(
PrDecl->getDeclContext()->getParent());

if (CxxRecord) {
return Ctx.getTypeDeclType(dyn_cast<TypeDecl>(CxxRecord));
}
}
}

return std::nullopt;
};

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

if (auto Type = ExplicitMemberHeuristic(Base)) {
BaseType = *Type;
}

BaseType = simplifyType(BaseType, Base, ME->isArrow());
return resolveDependentMember(BaseType, ME->getMember(), NoFilter);
}
Expand Down
19 changes: 18 additions & 1 deletion clang/unittests/Sema/HeuristicResolverTest.cpp
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