From: Sam McCall Date: Mon, 10 Jun 2019 15:17:52 +0000 (+0000) Subject: Re-land "[CodeComplete] Improve overload handling for C++ qualified and ref-qualified... X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=3dea52725862cc320d05056bfc433ad519fd5084;p=platform%2Fupstream%2Fllvm.git Re-land "[CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods." ShadowMapEntry is now really, truly a normal class. llvm-svn: 362950 --- diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index 4575d4f..d2e7766 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -16,7 +16,9 @@ #include "clang/AST/ExprCXX.h" #include "clang/AST/ExprObjC.h" #include "clang/AST/QualTypeNames.h" +#include "clang/AST/Type.h" #include "clang/Basic/CharInfo.h" +#include "clang/Basic/Specifiers.h" #include "clang/Lex/HeaderSearch.h" #include "clang/Lex/MacroInfo.h" #include "clang/Lex/Preprocessor.h" @@ -82,6 +84,15 @@ private: public: ShadowMapEntry() : DeclOrVector(), SingleDeclIndex(0) {} + ShadowMapEntry(const ShadowMapEntry &) = delete; + ShadowMapEntry(ShadowMapEntry &&Move) { *this = std::move(Move); } + ShadowMapEntry &operator=(const ShadowMapEntry &) = delete; + ShadowMapEntry &operator=(ShadowMapEntry &&Move) { + SingleDeclIndex = Move.SingleDeclIndex; + DeclOrVector = Move.DeclOrVector; + Move.DeclOrVector = nullptr; + return *this; + } void Add(const NamedDecl *ND, unsigned Index) { if (DeclOrVector.isNull()) { @@ -105,7 +116,7 @@ private: DeclIndexPair(ND, Index)); } - void Destroy() { + ~ShadowMapEntry() { if (DeclIndexPairVector *Vec = DeclOrVector.dyn_cast()) { delete Vec; @@ -152,9 +163,16 @@ private: /// different levels of, e.g., the inheritance hierarchy. std::list ShadowMaps; + /// Overloaded C++ member functions found by SemaLookup. + /// Used to determine when one overload is dominated by another. + llvm::DenseMap, ShadowMapEntry> + OverloadMap; + /// If we're potentially referring to a C++ member function, the set /// of qualifiers applied to the object type. Qualifiers ObjectTypeQualifiers; + /// The kind of the object expression, for rvalue/lvalue overloads. + ExprValueKind ObjectKind; /// Whether the \p ObjectTypeQualifiers field is active. bool HasObjectTypeQualifiers; @@ -230,8 +248,9 @@ public: /// out member functions that aren't available (because there will be a /// cv-qualifier mismatch) or prefer functions with an exact qualifier /// match. - void setObjectTypeQualifiers(Qualifiers Quals) { + void setObjectTypeQualifiers(Qualifiers Quals, ExprValueKind Kind) { ObjectTypeQualifiers = Quals; + ObjectKind = Kind; HasObjectTypeQualifiers = true; } @@ -1157,6 +1176,53 @@ static void setInBaseClass(ResultBuilder::Result &R) { R.InBaseClass = true; } +enum class OverloadCompare { BothViable, Dominates, Dominated }; +// Will Candidate ever be called on the object, when overloaded with Incumbent? +// Returns Dominates if Candidate is always called, Dominated if Incumbent is +// always called, BothViable if either may be called dependending on arguments. +// Precondition: must actually be overloads! +static OverloadCompare compareOverloads(const CXXMethodDecl &Candidate, + const CXXMethodDecl &Incumbent, + const Qualifiers &ObjectQuals, + ExprValueKind ObjectKind) { + if (Candidate.isVariadic() != Incumbent.isVariadic() || + Candidate.getNumParams() != Incumbent.getNumParams() || + Candidate.getMinRequiredArguments() != + Incumbent.getMinRequiredArguments()) + return OverloadCompare::BothViable; + for (unsigned I = 0, E = Candidate.getNumParams(); I != E; ++I) + if (Candidate.parameters()[I]->getType().getCanonicalType() != + Incumbent.parameters()[I]->getType().getCanonicalType()) + return OverloadCompare::BothViable; + if (!llvm::empty(Candidate.specific_attrs()) || + !llvm::empty(Incumbent.specific_attrs())) + return OverloadCompare::BothViable; + // At this point, we know calls can't pick one or the other based on + // arguments, so one of the two must win. (Or both fail, handled elsewhere). + RefQualifierKind CandidateRef = Candidate.getRefQualifier(); + RefQualifierKind IncumbentRef = Incumbent.getRefQualifier(); + if (CandidateRef != IncumbentRef) { + // If the object kind is LValue/RValue, there's one acceptable ref-qualifier + // and it can't be mixed with ref-unqualified overloads (in valid code). + + // For xvalue objects, we prefer the rvalue overload even if we have to + // add qualifiers (which is rare, because const&& is rare). + if (ObjectKind == clang::VK_XValue) + return CandidateRef == RQ_RValue ? OverloadCompare::Dominates + : OverloadCompare::Dominated; + } + // Now the ref qualifiers are the same (or we're in some invalid state). + // So make some decision based on the qualifiers. + Qualifiers CandidateQual = Candidate.getMethodQualifiers(); + Qualifiers IncumbentQual = Incumbent.getMethodQualifiers(); + bool CandidateSuperset = CandidateQual.compatiblyIncludes(IncumbentQual); + bool IncumbentSuperset = IncumbentQual.compatiblyIncludes(CandidateQual); + if (CandidateSuperset == IncumbentSuperset) + return OverloadCompare::BothViable; + return IncumbentSuperset ? OverloadCompare::Dominates + : OverloadCompare::Dominated; +} + void ResultBuilder::AddResult(Result R, DeclContext *CurContext, NamedDecl *Hiding, bool InBaseClass = false) { if (R.Kind != Result::RK_Declaration) { @@ -1233,6 +1299,44 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext, // qualifiers. return; } + // Detect cases where a ref-qualified method cannot be invoked. + switch (Method->getRefQualifier()) { + case RQ_LValue: + if (ObjectKind != VK_LValue && !MethodQuals.hasConst()) + return; + break; + case RQ_RValue: + if (ObjectKind == VK_LValue) + return; + break; + case RQ_None: + break; + } + + /// Check whether this dominates another overloaded method, which should + /// be suppressed (or vice versa). + /// Motivating case is const_iterator begin() const vs iterator begin(). + auto &OverloadSet = OverloadMap[std::make_pair( + CurContext, Method->getDeclName().getAsOpaqueInteger())]; + for (const DeclIndexPair& Entry : OverloadSet) { + Result &Incumbent = Results[Entry.second]; + switch (compareOverloads(*Method, + *cast(Incumbent.Declaration), + ObjectTypeQualifiers, ObjectKind)) { + case OverloadCompare::Dominates: + // Replace the dominated overload with this one. + // FIXME: if the overload dominates multiple incumbents then we + // should remove all. But two overloads is by far the common case. + Incumbent = std::move(R); + return; + case OverloadCompare::Dominated: + // This overload can't be called, drop it. + return; + case OverloadCompare::BothViable: + break; + } + } + OverloadSet.Add(Method, Results.size()); } // Insert this result into the set of results. @@ -1253,11 +1357,6 @@ void ResultBuilder::EnterNewScope() { ShadowMaps.emplace_back(); } /// Exit from the current scope. void ResultBuilder::ExitScope() { - for (ShadowMap::iterator E = ShadowMaps.back().begin(), - EEnd = ShadowMaps.back().end(); - E != EEnd; ++E) - E->second.Destroy(); - ShadowMaps.pop_back(); } @@ -3997,7 +4096,8 @@ void Sema::CodeCompleteOrdinaryName(Scope *S, // the member function to filter/prioritize the results list. auto ThisType = getCurrentThisType(); if (!ThisType.isNull()) - Results.setObjectTypeQualifiers(ThisType->getPointeeType().getQualifiers()); + Results.setObjectTypeQualifiers(ThisType->getPointeeType().getQualifiers(), + VK_LValue); CodeCompletionDeclConsumer Consumer(Results, CurContext); LookupVisibleDecls(S, LookupOrdinaryName, Consumer, @@ -4551,13 +4651,12 @@ AddObjCProperties(const CodeCompletionContext &CCContext, } } -static void -AddRecordMembersCompletionResults(Sema &SemaRef, ResultBuilder &Results, - Scope *S, QualType BaseType, RecordDecl *RD, - Optional AccessOpFixIt) { +static void AddRecordMembersCompletionResults( + Sema &SemaRef, ResultBuilder &Results, Scope *S, QualType BaseType, + ExprValueKind BaseKind, RecordDecl *RD, Optional AccessOpFixIt) { // Indicate that we are performing a member access, and the cv-qualifiers // for the base object type. - Results.setObjectTypeQualifiers(BaseType.getQualifiers()); + Results.setObjectTypeQualifiers(BaseType.getQualifiers(), BaseKind); // Access to a C/C++ class, struct, or union. Results.allowNestedNameSpecifiers(); @@ -4638,18 +4737,20 @@ void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base, Base = ConvertedBase.get(); QualType BaseType = Base->getType(); + ExprValueKind BaseKind = Base->getValueKind(); if (IsArrow) { - if (const PointerType *Ptr = BaseType->getAs()) + if (const PointerType *Ptr = BaseType->getAs()) { BaseType = Ptr->getPointeeType(); - else if (BaseType->isObjCObjectPointerType()) + BaseKind = VK_LValue; + } else if (BaseType->isObjCObjectPointerType()) /*Do nothing*/; else return false; } if (const RecordType *Record = BaseType->getAs()) { - AddRecordMembersCompletionResults(*this, Results, S, BaseType, + AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind, Record->getDecl(), std::move(AccessOpFixIt)); } else if (const auto *TST = @@ -4658,13 +4759,13 @@ void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base, if (const auto *TD = dyn_cast_or_null(TN.getAsTemplateDecl())) { CXXRecordDecl *RD = TD->getTemplatedDecl(); - AddRecordMembersCompletionResults(*this, Results, S, BaseType, RD, - std::move(AccessOpFixIt)); + AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind, + RD, std::move(AccessOpFixIt)); } } else if (const auto *ICNT = BaseType->getAs()) { if (auto *RD = ICNT->getDecl()) - AddRecordMembersCompletionResults(*this, Results, S, BaseType, RD, - std::move(AccessOpFixIt)); + AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind, + RD, std::move(AccessOpFixIt)); } else if (!IsArrow && BaseType->isObjCObjectPointerType()) { // Objective-C property reference. AddedPropertiesSet AddedProperties; diff --git a/clang/test/CodeCompletion/member-access.cpp b/clang/test/CodeCompletion/member-access.cpp index 003d224..6d7cf6a 100644 --- a/clang/test/CodeCompletion/member-access.cpp +++ b/clang/test/CodeCompletion/member-access.cpp @@ -210,3 +210,66 @@ void test3(const Proxy2 &p) { // CHECK-CC9: memfun2 (InBase) : [#void#][#Base3::#]memfun2(<#int#>) (requires fix-it: {181:4-181:5} to "->") // CHECK-CC9: memfun3 : [#int#]memfun3(<#int#>) (requires fix-it: {181:4-181:5} to "->") // CHECK-CC9: operator-> : [#Derived *#]operator->()[# const#] + +// These overload sets differ only by return type and this-qualifiers. +// So for any given callsite, only one is available. +struct Overloads { + double ConstOverload(char); + int ConstOverload(char) const; + + int RefOverload(char) &; + double RefOverload(char) const&; + char RefOverload(char) &&; +}; +void testLValue(Overloads& Ref) { + Ref. +} +void testConstLValue(const Overloads& ConstRef) { + ConstRef. +} +void testRValue() { + Overloads(). +} +void testXValue(Overloads& X) { + static_cast(X). +} + +// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:225:7 %s -o - | FileCheck -check-prefix=CHECK-LVALUE %s \ +// RUN: --implicit-check-not="[#int#]ConstOverload(" \ +// RUN: --implicit-check-not="[#double#]RefOverload(" \ +// RUN: --implicit-check-not="[#char#]RefOverload(" +// CHECK-LVALUE-DAG: [#double#]ConstOverload( +// CHECK-LVALUE-DAG: [#int#]RefOverload( + +// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:228:12 %s -o - | FileCheck -check-prefix=CHECK-CONSTLVALUE %s \ +// RUN: --implicit-check-not="[#double#]ConstOverload(" \ +// RUN: --implicit-check-not="[#int#]RefOverload(" \ +// RUN: --implicit-check-not="[#char#]RefOverload(" +// CHECK-CONSTLVALUE: [#int#]ConstOverload( +// CHECK-CONSTLVALUE: [#double#]RefOverload( + +// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:231:15 %s -o - | FileCheck -check-prefix=CHECK-PRVALUE %s \ +// RUN: --implicit-check-not="[#int#]ConstOverload(" \ +// RUN: --implicit-check-not="[#int#]RefOverload(" \ +// RUN: --implicit-check-not="[#double#]RefOverload(" +// CHECK-PRVALUE: [#double#]ConstOverload( +// CHECK-PRVALUE: [#char#]RefOverload( + +// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:234:31 %s -o - | FileCheck -check-prefix=CHECK-XVALUE %s \ +// RUN: --implicit-check-not="[#int#]ConstOverload(" \ +// RUN: --implicit-check-not="[#int#]RefOverload(" \ +// RUN: --implicit-check-not="[#double#]RefOverload(" +// CHECK-XVALUE: [#double#]ConstOverload( +// CHECK-XVALUE: [#char#]RefOverload( + +void testOverloadOperator() { + struct S { + char operator=(int) const; + int operator=(int); + } s; + return s. +} +// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:270:12 %s -o - | FileCheck -check-prefix=CHECK-OPER %s \ +// RUN: --implicit-check-not="[#char#]operator=(" +// CHECK-OPER: [#int#]operator=( +