r362785 - [CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Jun 10 02:49:18 PDT 2019
Thanks! Re-landed in r362924.
On Fri, Jun 7, 2019 at 9:15 PM Vlad Tsyrklevich via cfe-commits <
cfe-commits at 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 at 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 at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190610/3aec7f07/attachment-0001.html>
More information about the cfe-commits
mailing list