zyounan updated this revision to Diff 545920.
zyounan marked 2 inline comments as done.
zyounan added a comment.
Herald added a project: clang.
Update
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D156300/new/
https://reviews.llvm.org/D156300
Files:
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp
clang/lib/Sema/SemaCodeComplete.cpp
clang/unittests/Sema/CodeCompleteTest.cpp
Index: clang/unittests/Sema/CodeCompleteTest.cpp
===================================================================
--- clang/unittests/Sema/CodeCompleteTest.cpp
+++ clang/unittests/Sema/CodeCompleteTest.cpp
@@ -214,15 +214,25 @@
struct OtherClass {
OtherClass() {
Foo f;
+ Derived d;
f.$canBeCall^
+ ; // Prevent parsing as 'f.f'
+ f.Foo::$canBeCall^
&Foo::$cannotBeCall^
+ ;
+ d.Foo::$canBeCall^
}
};
int main() {
Foo f;
+ Derived d;
f.$canBeCall^
+ ; // Prevent parsing as 'f.f'
+ f.Foo::$canBeCall^
&Foo::$cannotBeCall^
+ ;
+ d.Foo::$canBeCall^
}
)cpp");
Index: clang/lib/Sema/SemaCodeComplete.cpp
===================================================================
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -338,8 +338,11 @@
///
/// \param InBaseClass whether the result was found in a base
/// class of the searched context.
+ ///
+ /// \param BaseType the type of expression that precedes the "." or "->"
+ /// in a member access expression.
void AddResult(Result R, DeclContext *CurContext, NamedDecl *Hiding,
- bool InBaseClass);
+ bool InBaseClass, QualType BaseType);
/// Add a new non-declaration result to this result set.
void AddResult(Result R);
@@ -1262,7 +1265,8 @@
}
void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
- NamedDecl *Hiding, bool InBaseClass = false) {
+ NamedDecl *Hiding, bool InBaseClass = false,
+ QualType BaseType = QualType()) {
if (R.Kind != Result::RK_Declaration) {
// For non-declaration results, just add the result.
Results.push_back(R);
@@ -1380,9 +1384,7 @@
OverloadSet.Add(Method, Results.size());
}
- // When completing a non-static member function (and not via
- // dot/arrow member access) and we're not inside that class' scope,
- // it can't be a call.
+ // Decide whether or not a non-static member function can be a call.
if (CompletionContext.getKind() == clang::CodeCompletionContext::CCC_Symbol) {
const NamedDecl *ND = R.getDeclaration();
if (const auto *FuncTmpl = dyn_cast<FunctionTemplateDecl>(ND)) {
@@ -1404,10 +1406,24 @@
return nullptr;
}();
+ // When completing a non-static member function (and not via
+ // dot/arrow member access) and we're not inside that class' scope,
+ // it can't be a call.
R.FunctionCanBeCall =
CurrentClassScope &&
(CurrentClassScope == Method->getParent() ||
CurrentClassScope->isDerivedFrom(Method->getParent()));
+
+ // If the member access "." or "->" is followed by a qualified Id and the
+ // object type is derived from or the same as that of the Id, then
+ // the candidate functions should be perceived as calls.
+ if (const CXXRecordDecl *MaybeDerived = nullptr;
+ !BaseType.isNull() &&
+ (MaybeDerived = BaseType->getAsCXXRecordDecl())) {
+ auto *MaybeBase = Method->getParent();
+ R.FunctionCanBeCall =
+ MaybeDerived == MaybeBase || MaybeDerived->isDerivedFrom(MaybeBase);
+ }
}
}
@@ -1683,7 +1699,7 @@
bool InBaseClass) override {
ResultBuilder::Result Result(ND, Results.getBasePriority(ND), nullptr,
false, IsAccessible(ND, Ctx), FixIts);
- Results.AddResult(Result, InitialLookupCtx, Hiding, InBaseClass);
+ Results.AddResult(Result, InitialLookupCtx, Hiding, InBaseClass, BaseType);
}
void EnteredContext(DeclContext *Ctx) override {
Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -81,11 +81,13 @@
}
template <typename... ExpectedHints>
-void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
- ExpectedHints... Expected) {
+void assertHintsWithHeader(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
+ llvm::StringRef HeaderContent,
+ ExpectedHints... Expected) {
Annotations Source(AnnotatedSource);
TestTU TU = TestTU::withCode(Source.code());
TU.ExtraArgs.push_back("-std=c++20");
+ TU.HeaderCode = HeaderContent;
auto AST = TU.build();
EXPECT_THAT(hintsOfKind(AST, Kind),
@@ -96,6 +98,13 @@
EXPECT_THAT(inlayHints(AST, std::nullopt), IsEmpty());
}
+template <typename... ExpectedHints>
+void assertHints(InlayHintKind Kind, llvm::StringRef AnnotatedSource,
+ ExpectedHints... Expected) {
+ return assertHintsWithHeader(Kind, AnnotatedSource, "",
+ std::move(Expected)...);
+}
+
// Hack to allow expression-statements operating on parameter packs in C++14.
template <typename... T> void ignore(T &&...) {}
@@ -1421,8 +1430,7 @@
}
TEST(TypeHints, SubstTemplateParameterAliases) {
- assertTypeHints(
- R"cpp(
+ llvm::StringRef Header = R"cpp(
template <class T> struct allocator {};
template <class T, class A>
@@ -1455,9 +1463,34 @@
T elements[10];
};
+ )cpp";
+
+ llvm::StringRef VectorIntPtr = R"cpp(
+ vector<int *> array;
+ auto $no_modifier[[x]] = array[3];
+ auto* $ptr_modifier[[ptr]] = &array[3];
+ auto& $ref_modifier[[ref]] = array[3];
+ auto& $at[[immutable]] = array.at(3);
+
+ auto $data[[data]] = array.data();
+ auto $allocator[[alloc]] = array.get_allocator();
+ auto $size[[size]] = array.size();
+ auto $begin[[begin]] = array.begin();
+ auto $end[[end]] = array.end();
+ )cpp";
+
+ assertHintsWithHeader(
+ InlayHintKind::Type, VectorIntPtr, Header,
+ ExpectedHint{": int *", "no_modifier"},
+ ExpectedHint{": int **", "ptr_modifier"},
+ ExpectedHint{": int *&", "ref_modifier"},
+ ExpectedHint{": int *const &", "at"}, ExpectedHint{": int **", "data"},
+ ExpectedHint{": allocator<int *>", "allocator"},
+ ExpectedHint{": size_type", "size"}, ExpectedHint{": iterator", "begin"},
+ ExpectedHint{": non_template_iterator", "end"});
+ llvm::StringRef VectorInt = R"cpp(
vector<int> array;
-
auto $no_modifier[[by_value]] = array[3];
auto* $ptr_modifier[[ptr]] = &array[3];
auto& $ref_modifier[[ref]] = array[3];
@@ -1468,8 +1501,19 @@
auto $size[[size]] = array.size();
auto $begin[[begin]] = array.begin();
auto $end[[end]] = array.end();
+ )cpp";
+ assertHintsWithHeader(
+ InlayHintKind::Type, VectorInt, Header,
+ ExpectedHint{": int", "no_modifier"},
+ ExpectedHint{": int *", "ptr_modifier"},
+ ExpectedHint{": int &", "ref_modifier"},
+ ExpectedHint{": const int &", "at"}, ExpectedHint{": int *", "data"},
+ ExpectedHint{": allocator<int>", "allocator"},
+ ExpectedHint{": size_type", "size"}, ExpectedHint{": iterator", "begin"},
+ ExpectedHint{": non_template_iterator", "end"});
+ llvm::StringRef TypeAlias = R"cpp(
// If the type alias is not of substituted template parameter type,
// do not show desugared type.
using VeryLongLongTypeName = my_iterator;
@@ -1484,16 +1528,11 @@
using static_vector = basic_static_vector<T, allocator<T>>;
auto $vector_name[[vec]] = static_vector<int>();
- )cpp",
- ExpectedHint{": int", "no_modifier"},
- ExpectedHint{": int *", "ptr_modifier"},
- ExpectedHint{": int &", "ref_modifier"},
- ExpectedHint{": const int &", "at"}, ExpectedHint{": int *", "data"},
- ExpectedHint{": allocator<int>", "allocator"},
- ExpectedHint{": size_type", "size"}, ExpectedHint{": iterator", "begin"},
- ExpectedHint{": non_template_iterator", "end"},
- ExpectedHint{": Short", "short_name"},
- ExpectedHint{": static_vector<int>", "vector_name"});
+ )cpp";
+
+ assertHintsWithHeader(InlayHintKind::Type, TypeAlias, Header,
+ ExpectedHint{": Short", "short_name"},
+ ExpectedHint{": static_vector<int>", "vector_name"});
}
TEST(DesignatorHints, Basic) {
Index: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -552,15 +552,25 @@
struct OtherClass {
OtherClass() {
Foo f;
+ Derived d;
f.$canBeCall^
+ ; // Prevent parsing as 'f.f'
+ f.Foo::$canBeCall^
&Foo::$canNotBeCall^
+ ;
+ d.Foo::$canBeCall^
}
};
int main() {
Foo f;
+ Derived d;
f.$canBeCall^
+ ; // Prevent parsing as 'f.f'
+ f.Foo::$canBeCall^
&Foo::$canNotBeCall^
+ ;
+ d.Foo::$canBeCall^
}
)cpp");
auto TU = TestTU::withCode(Code.code());
Index: clang-tools-extra/clangd/InlayHints.cpp
===================================================================
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -195,22 +195,47 @@
// Determines if any intermediate type in desugaring QualType QT is of
// substituted template parameter type. Ignore pointer or reference wrappers.
bool isSugaredTemplateParameter(QualType QT) {
- static auto PeelWrappers = [](QualType QT) {
+ static auto PeelWrapper = [](QualType QT) {
// Neither `PointerType` nor `ReferenceType` is considered as sugared
// type. Peel it.
- QualType Next;
- while (!(Next = QT->getPointeeType()).isNull())
- QT = Next;
- return QT;
+ QualType Next = QT->getPointeeType();
+ return Next.isNull() ? QT : Next;
};
+
+ // This is a bit tricky: we traverse the type structure and find whether or
+ // not a type in the desugaring process is of SubstTemplateTypeParmType.
+ // During the process, we may encounter pointer or reference types that are
+ // not marked as sugared; therefore, the desugar function won't apply. To
+ // move forward the traversal, we retrieve the pointees using
+ // QualType::getPointeeType().
+ //
+ // However, getPointeeType could leap over our interests: The QT::getAs<T>()
+ // invoked would implicitly desugar the type. Consequently, if the
+ // SubstTemplateTypeParmType is encompassed within a TypedefType, we may lose
+ // the chance to visit it.
+ // For example, given a QT that represents `std::vector<int *>::value_type`:
+ // `-ElaboratedType 'value_type' sugar
+ // `-TypedefType 'vector<int *>::value_type' sugar
+ // |-Typedef 'value_type'
+ // `-SubstTemplateTypeParmType 'int *' sugar class depth 0 index 0 T
+ // |-ClassTemplateSpecialization 'vector'
+ // `-PointerType 'int *'
+ // `-BuiltinType 'int'
+ // Applying `getPointeeType` to QT results in 'int', a child of our target
+ // node SubstTemplateTypeParmType.
+ //
+ // As such, we always prefer the desugared over the pointee for next type
+ // in the iteration. It could avoid the getPointeeType's implicit desugaring.
while (true) {
- QualType Desugared =
- PeelWrappers(QT->getLocallyUnqualifiedSingleStepDesugaredType());
- if (Desugared == QT)
- break;
- if (Desugared->getAs<SubstTemplateTypeParmType>())
+ if (QT->getAs<SubstTemplateTypeParmType>())
return true;
- QT = Desugared;
+ QualType Desugared = QT->getLocallyUnqualifiedSingleStepDesugaredType();
+ if (Desugared != QT)
+ QT = Desugared;
+ else if (auto Peeled = PeelWrapper(Desugared); Peeled != QT)
+ QT = Peeled;
+ else
+ break;
}
return false;
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
