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
