Index: lib/Sema/SemaCodeComplete.cpp =================================================================== --- lib/Sema/SemaCodeComplete.cpp +++ 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" @@ -152,9 +154,16 @@ /// 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 +239,9 @@ /// 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 +1167,45 @@ R.InBaseClass = true; } +// Will Object.Candidate() always be called instead of Object.Incumbent()? +// Precondition: must have the same name, and not be a template. +static bool overloadDominates(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 false; + for (unsigned I = 0, E = Candidate.getNumParams(); I != E; ++I) + if (Candidate.parameters()[I]->getType().getCanonicalType() != + Incumbent.parameters()[I]->getType().getCanonicalType()) + return false; + if (!Candidate.specific_attrs().empty() || + !Incumbent.specific_attrs().empty()) + return false; + // 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; + } + // 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(); + return IncumbentQual.compatiblyIncludes(CandidateQual) && + !CandidateQual.compatiblyIncludes(IncumbentQual); +} + void ResultBuilder::AddResult(Result R, DeclContext *CurContext, NamedDecl *Hiding, bool InBaseClass = false) { if (R.Kind != Result::RK_Declaration) { @@ -1233,6 +1282,34 @@ // 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; + } + + auto &OverloadSet = + OverloadMap[std::make_pair(CurContext, Method->getName())]; + for (const DeclIndexPair& Entry : OverloadSet) { + Result &Incumbent = Results[Entry.second]; + if (overloadDominates(*Method, + *cast(Incumbent.Declaration), + ObjectTypeQualifiers, ObjectKind)) { + // 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; + } + } + OverloadSet.Add(Method, Results.size()); } // Insert this result into the set of results. @@ -3984,7 +4061,8 @@ // 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, @@ -4538,13 +4616,12 @@ } } -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(); @@ -4625,18 +4702,20 @@ 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 = @@ -4645,13 +4724,13 @@ 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; Index: test/CodeCompletion/member-access.cpp =================================================================== --- test/CodeCompletion/member-access.cpp +++ test/CodeCompletion/member-access.cpp @@ -210,3 +210,54 @@ // 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 { + int ConstOverload(char) const; + double ConstOverload(char); + + 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(