r362785 - [CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods.
Vlad Tsyrklevich via cfe-commits
cfe-commits at lists.llvm.org
Fri Jun 7 12:15:35 PDT 2019
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190607/f7fc47da/attachment-0001.html>
More information about the cfe-commits
mailing list