Thanks! Re-landed in r362924. On Fri, Jun 7, 2019 at 9:15 PM Vlad Tsyrklevich via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> This change caused LSan failures and has been reverted in r362830: > http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/32809 > > On Fri, Jun 7, 2019 at 2:42 AM Sam McCall via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: sammccall >> Date: Fri Jun 7 02:45:17 2019 >> New Revision: 362785 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=362785&view=rev >> Log: >> [CodeComplete] Improve overload handling for C++ qualified and >> ref-qualified methods. >> >> Summary: >> - when a method is not available because of the target value kind (e.g. >> an && >> method on a Foo& variable), then don't offer it. >> - when a method is effectively shadowed by another method from the same >> class >> with a) an identical argument list and b) superior qualifiers, then >> don't >> offer it. >> >> Reviewers: ilya-biryukov >> >> Subscribers: cfe-commits >> >> Tags: #clang >> >> Differential Revision: https://reviews.llvm.org/D62582 >> >> Modified: >> cfe/trunk/lib/Sema/SemaCodeComplete.cpp >> cfe/trunk/test/CodeCompletion/member-access.cpp >> >> Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=362785&r1=362784&r2=362785&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Fri Jun 7 02:45:17 2019 >> @@ -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 @@ private: >> /// different levels of, e.g., the inheritance hierarchy. >> std::list<ShadowMap> ShadowMaps; >> >> + /// Overloaded C++ member functions found by SemaLookup. >> + /// Used to determine when one overload is dominated by another. >> + llvm::DenseMap<std::pair<DeclContext *, /*Name*/uintptr_t>, >> 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 @@ 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 +1167,53 @@ static void setInBaseClass(ResultBuilder >> 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<EnableIfAttr>()) || >> + !llvm::empty(Incumbent.specific_attrs<EnableIfAttr>())) >> + 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 +1290,44 @@ void ResultBuilder::AddResult(Result R, >> // 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<CXXMethodDecl>(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. >> @@ -3997,7 +4092,8 @@ void Sema::CodeCompleteOrdinaryName(Scop >> // 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 +4647,12 @@ AddObjCProperties(const CodeCompletionCo >> } >> } >> >> -static void >> -AddRecordMembersCompletionResults(Sema &SemaRef, ResultBuilder &Results, >> - Scope *S, QualType BaseType, >> RecordDecl *RD, >> - Optional<FixItHint> AccessOpFixIt) { >> +static void AddRecordMembersCompletionResults( >> + Sema &SemaRef, ResultBuilder &Results, Scope *S, QualType BaseType, >> + ExprValueKind BaseKind, RecordDecl *RD, Optional<FixItHint> >> 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 +4733,20 @@ void Sema::CodeCompleteMemberReferenceEx >> Base = ConvertedBase.get(); >> >> QualType BaseType = Base->getType(); >> + ExprValueKind BaseKind = Base->getValueKind(); >> >> if (IsArrow) { >> - if (const PointerType *Ptr = BaseType->getAs<PointerType>()) >> + if (const PointerType *Ptr = BaseType->getAs<PointerType>()) { >> 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<RecordType>()) { >> - AddRecordMembersCompletionResults(*this, Results, S, BaseType, >> + AddRecordMembersCompletionResults(*this, Results, S, BaseType, >> BaseKind, >> Record->getDecl(), >> std::move(AccessOpFixIt)); >> } else if (const auto *TST = >> @@ -4658,13 +4755,13 @@ void Sema::CodeCompleteMemberReferenceEx >> if (const auto *TD = >> >> dyn_cast_or_null<ClassTemplateDecl>(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<InjectedClassNameType>()) { >> 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; >> >> Modified: cfe/trunk/test/CodeCompletion/member-access.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/member-access.cpp?rev=362785&r1=362784&r2=362785&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/CodeCompletion/member-access.cpp (original) >> +++ cfe/trunk/test/CodeCompletion/member-access.cpp Fri Jun 7 02:45:17 >> 2019 >> @@ -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<Overloads&&>(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=( >> + >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
