<div dir="ltr">Thanks! Re-landed in r362924.</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jun 7, 2019 at 9:15 PM Vlad Tsyrklevich via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr">This change caused LSan failures and has been reverted in r362830: <a href="http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/32809" target="_blank">http://lab.llvm.org:8011/builders/sanitizer-x86_64-linux-fast/builds/32809</a></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jun 7, 2019 at 2:42 AM Sam McCall via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: sammccall<br>
Date: Fri Jun 7 02:45:17 2019<br>
New Revision: 362785<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=362785&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=362785&view=rev</a><br>
Log:<br>
[CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods.<br>
<br>
Summary:<br>
- when a method is not available because of the target value kind (e.g. an &&<br>
method on a Foo& variable), then don't offer it.<br>
- when a method is effectively shadowed by another method from the same class<br>
with a) an identical argument list and b) superior qualifiers, then don't<br>
offer it.<br>
<br>
Reviewers: ilya-biryukov<br>
<br>
Subscribers: cfe-commits<br>
<br>
Tags: #clang<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D62582" rel="noreferrer" target="_blank">https://reviews.llvm.org/D62582</a><br>
<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaCodeComplete.cpp<br>
cfe/trunk/test/CodeCompletion/member-access.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaCodeComplete.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=362785&r1=362784&r2=362785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCodeComplete.cpp?rev=362785&r1=362784&r2=362785&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaCodeComplete.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaCodeComplete.cpp Fri Jun 7 02:45:17 2019<br>
@@ -16,7 +16,9 @@<br>
#include "clang/AST/ExprCXX.h"<br>
#include "clang/AST/ExprObjC.h"<br>
#include "clang/AST/QualTypeNames.h"<br>
+#include "clang/AST/Type.h"<br>
#include "clang/Basic/CharInfo.h"<br>
+#include "clang/Basic/Specifiers.h"<br>
#include "clang/Lex/HeaderSearch.h"<br>
#include "clang/Lex/MacroInfo.h"<br>
#include "clang/Lex/Preprocessor.h"<br>
@@ -152,9 +154,16 @@ private:<br>
/// different levels of, e.g., the inheritance hierarchy.<br>
std::list<ShadowMap> ShadowMaps;<br>
<br>
+ /// Overloaded C++ member functions found by SemaLookup.<br>
+ /// Used to determine when one overload is dominated by another.<br>
+ llvm::DenseMap<std::pair<DeclContext *, /*Name*/uintptr_t>, ShadowMapEntry><br>
+ OverloadMap;<br>
+<br>
/// If we're potentially referring to a C++ member function, the set<br>
/// of qualifiers applied to the object type.<br>
Qualifiers ObjectTypeQualifiers;<br>
+ /// The kind of the object expression, for rvalue/lvalue overloads.<br>
+ ExprValueKind ObjectKind;<br>
<br>
/// Whether the \p ObjectTypeQualifiers field is active.<br>
bool HasObjectTypeQualifiers;<br>
@@ -230,8 +239,9 @@ public:<br>
/// out member functions that aren't available (because there will be a<br>
/// cv-qualifier mismatch) or prefer functions with an exact qualifier<br>
/// match.<br>
- void setObjectTypeQualifiers(Qualifiers Quals) {<br>
+ void setObjectTypeQualifiers(Qualifiers Quals, ExprValueKind Kind) {<br>
ObjectTypeQualifiers = Quals;<br>
+ ObjectKind = Kind;<br>
HasObjectTypeQualifiers = true;<br>
}<br>
<br>
@@ -1157,6 +1167,53 @@ static void setInBaseClass(ResultBuilder<br>
R.InBaseClass = true;<br>
}<br>
<br>
+enum class OverloadCompare { BothViable, Dominates, Dominated };<br>
+// Will Candidate ever be called on the object, when overloaded with Incumbent?<br>
+// Returns Dominates if Candidate is always called, Dominated if Incumbent is<br>
+// always called, BothViable if either may be called dependending on arguments.<br>
+// Precondition: must actually be overloads!<br>
+static OverloadCompare compareOverloads(const CXXMethodDecl &Candidate,<br>
+ const CXXMethodDecl &Incumbent,<br>
+ const Qualifiers &ObjectQuals,<br>
+ ExprValueKind ObjectKind) {<br>
+ if (Candidate.isVariadic() != Incumbent.isVariadic() ||<br>
+ Candidate.getNumParams() != Incumbent.getNumParams() ||<br>
+ Candidate.getMinRequiredArguments() !=<br>
+ Incumbent.getMinRequiredArguments())<br>
+ return OverloadCompare::BothViable;<br>
+ for (unsigned I = 0, E = Candidate.getNumParams(); I != E; ++I)<br>
+ if (Candidate.parameters()[I]->getType().getCanonicalType() !=<br>
+ Incumbent.parameters()[I]->getType().getCanonicalType())<br>
+ return OverloadCompare::BothViable;<br>
+ if (!llvm::empty(Candidate.specific_attrs<EnableIfAttr>()) ||<br>
+ !llvm::empty(Incumbent.specific_attrs<EnableIfAttr>()))<br>
+ return OverloadCompare::BothViable;<br>
+ // At this point, we know calls can't pick one or the other based on<br>
+ // arguments, so one of the two must win. (Or both fail, handled elsewhere).<br>
+ RefQualifierKind CandidateRef = Candidate.getRefQualifier();<br>
+ RefQualifierKind IncumbentRef = Incumbent.getRefQualifier();<br>
+ if (CandidateRef != IncumbentRef) {<br>
+ // If the object kind is LValue/RValue, there's one acceptable ref-qualifier<br>
+ // and it can't be mixed with ref-unqualified overloads (in valid code).<br>
+<br>
+ // For xvalue objects, we prefer the rvalue overload even if we have to<br>
+ // add qualifiers (which is rare, because const&& is rare).<br>
+ if (ObjectKind == clang::VK_XValue)<br>
+ return CandidateRef == RQ_RValue ? OverloadCompare::Dominates<br>
+ : OverloadCompare::Dominated;<br>
+ }<br>
+ // Now the ref qualifiers are the same (or we're in some invalid state).<br>
+ // So make some decision based on the qualifiers.<br>
+ Qualifiers CandidateQual = Candidate.getMethodQualifiers();<br>
+ Qualifiers IncumbentQual = Incumbent.getMethodQualifiers();<br>
+ bool CandidateSuperset = CandidateQual.compatiblyIncludes(IncumbentQual);<br>
+ bool IncumbentSuperset = IncumbentQual.compatiblyIncludes(CandidateQual);<br>
+ if (CandidateSuperset == IncumbentSuperset)<br>
+ return OverloadCompare::BothViable;<br>
+ return IncumbentSuperset ? OverloadCompare::Dominates<br>
+ : OverloadCompare::Dominated;<br>
+}<br>
+<br>
void ResultBuilder::AddResult(Result R, DeclContext *CurContext,<br>
NamedDecl *Hiding, bool InBaseClass = false) {<br>
if (R.Kind != Result::RK_Declaration) {<br>
@@ -1233,6 +1290,44 @@ void ResultBuilder::AddResult(Result R,<br>
// qualifiers.<br>
return;<br>
}<br>
+ // Detect cases where a ref-qualified method cannot be invoked.<br>
+ switch (Method->getRefQualifier()) {<br>
+ case RQ_LValue:<br>
+ if (ObjectKind != VK_LValue && !MethodQuals.hasConst())<br>
+ return;<br>
+ break;<br>
+ case RQ_RValue:<br>
+ if (ObjectKind == VK_LValue)<br>
+ return;<br>
+ break;<br>
+ case RQ_None:<br>
+ break;<br>
+ }<br>
+<br>
+ /// Check whether this dominates another overloaded method, which should<br>
+ /// be suppressed (or vice versa).<br>
+ /// Motivating case is const_iterator begin() const vs iterator begin().<br>
+ auto &OverloadSet = OverloadMap[std::make_pair(<br>
+ CurContext, Method->getDeclName().getAsOpaqueInteger())];<br>
+ for (const DeclIndexPair& Entry : OverloadSet) {<br>
+ Result &Incumbent = Results[Entry.second];<br>
+ switch (compareOverloads(*Method,<br>
+ *cast<CXXMethodDecl>(Incumbent.Declaration),<br>
+ ObjectTypeQualifiers, ObjectKind)) {<br>
+ case OverloadCompare::Dominates:<br>
+ // Replace the dominated overload with this one.<br>
+ // FIXME: if the overload dominates multiple incumbents then we<br>
+ // should remove all. But two overloads is by far the common case.<br>
+ Incumbent = std::move(R);<br>
+ return;<br>
+ case OverloadCompare::Dominated:<br>
+ // This overload can't be called, drop it.<br>
+ return;<br>
+ case OverloadCompare::BothViable:<br>
+ break;<br>
+ }<br>
+ }<br>
+ OverloadSet.Add(Method, Results.size());<br>
}<br>
<br>
// Insert this result into the set of results.<br>
@@ -3997,7 +4092,8 @@ void Sema::CodeCompleteOrdinaryName(Scop<br>
// the member function to filter/prioritize the results list.<br>
auto ThisType = getCurrentThisType();<br>
if (!ThisType.isNull())<br>
- Results.setObjectTypeQualifiers(ThisType->getPointeeType().getQualifiers());<br>
+ Results.setObjectTypeQualifiers(ThisType->getPointeeType().getQualifiers(),<br>
+ VK_LValue);<br>
<br>
CodeCompletionDeclConsumer Consumer(Results, CurContext);<br>
LookupVisibleDecls(S, LookupOrdinaryName, Consumer,<br>
@@ -4551,13 +4647,12 @@ AddObjCProperties(const CodeCompletionCo<br>
}<br>
}<br>
<br>
-static void<br>
-AddRecordMembersCompletionResults(Sema &SemaRef, ResultBuilder &Results,<br>
- Scope *S, QualType BaseType, RecordDecl *RD,<br>
- Optional<FixItHint> AccessOpFixIt) {<br>
+static void AddRecordMembersCompletionResults(<br>
+ Sema &SemaRef, ResultBuilder &Results, Scope *S, QualType BaseType,<br>
+ ExprValueKind BaseKind, RecordDecl *RD, Optional<FixItHint> AccessOpFixIt) {<br>
// Indicate that we are performing a member access, and the cv-qualifiers<br>
// for the base object type.<br>
- Results.setObjectTypeQualifiers(BaseType.getQualifiers());<br>
+ Results.setObjectTypeQualifiers(BaseType.getQualifiers(), BaseKind);<br>
<br>
// Access to a C/C++ class, struct, or union.<br>
Results.allowNestedNameSpecifiers();<br>
@@ -4638,18 +4733,20 @@ void Sema::CodeCompleteMemberReferenceEx<br>
Base = ConvertedBase.get();<br>
<br>
QualType BaseType = Base->getType();<br>
+ ExprValueKind BaseKind = Base->getValueKind();<br>
<br>
if (IsArrow) {<br>
- if (const PointerType *Ptr = BaseType->getAs<PointerType>())<br>
+ if (const PointerType *Ptr = BaseType->getAs<PointerType>()) {<br>
BaseType = Ptr->getPointeeType();<br>
- else if (BaseType->isObjCObjectPointerType())<br>
+ BaseKind = VK_LValue;<br>
+ } else if (BaseType->isObjCObjectPointerType())<br>
/*Do nothing*/;<br>
else<br>
return false;<br>
}<br>
<br>
if (const RecordType *Record = BaseType->getAs<RecordType>()) {<br>
- AddRecordMembersCompletionResults(*this, Results, S, BaseType,<br>
+ AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind,<br>
Record->getDecl(),<br>
std::move(AccessOpFixIt));<br>
} else if (const auto *TST =<br>
@@ -4658,13 +4755,13 @@ void Sema::CodeCompleteMemberReferenceEx<br>
if (const auto *TD =<br>
dyn_cast_or_null<ClassTemplateDecl>(TN.getAsTemplateDecl())) {<br>
CXXRecordDecl *RD = TD->getTemplatedDecl();<br>
- AddRecordMembersCompletionResults(*this, Results, S, BaseType, RD,<br>
- std::move(AccessOpFixIt));<br>
+ AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind,<br>
+ RD, std::move(AccessOpFixIt));<br>
}<br>
} else if (const auto *ICNT = BaseType->getAs<InjectedClassNameType>()) {<br>
if (auto *RD = ICNT->getDecl())<br>
- AddRecordMembersCompletionResults(*this, Results, S, BaseType, RD,<br>
- std::move(AccessOpFixIt));<br>
+ AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind,<br>
+ RD, std::move(AccessOpFixIt));<br>
} else if (!IsArrow && BaseType->isObjCObjectPointerType()) {<br>
// Objective-C property reference.<br>
AddedPropertiesSet AddedProperties;<br>
<br>
Modified: cfe/trunk/test/CodeCompletion/member-access.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/member-access.cpp?rev=362785&r1=362784&r2=362785&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeCompletion/member-access.cpp?rev=362785&r1=362784&r2=362785&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/CodeCompletion/member-access.cpp (original)<br>
+++ cfe/trunk/test/CodeCompletion/member-access.cpp Fri Jun 7 02:45:17 2019<br>
@@ -210,3 +210,66 @@ void test3(const Proxy2 &p) {<br>
// CHECK-CC9: memfun2 (InBase) : [#void#][#Base3::#]memfun2(<#int#>) (requires fix-it: {181:4-181:5} to "->")<br>
// CHECK-CC9: memfun3 : [#int#]memfun3(<#int#>) (requires fix-it: {181:4-181:5} to "->")<br>
// CHECK-CC9: operator-> : [#Derived *#]operator->()[# const#]<br>
+<br>
+// These overload sets differ only by return type and this-qualifiers.<br>
+// So for any given callsite, only one is available.<br>
+struct Overloads {<br>
+ double ConstOverload(char);<br>
+ int ConstOverload(char) const;<br>
+<br>
+ int RefOverload(char) &;<br>
+ double RefOverload(char) const&;<br>
+ char RefOverload(char) &&;<br>
+};<br>
+void testLValue(Overloads& Ref) {<br>
+ Ref.<br>
+}<br>
+void testConstLValue(const Overloads& ConstRef) {<br>
+ ConstRef.<br>
+}<br>
+void testRValue() {<br>
+ Overloads().<br>
+}<br>
+void testXValue(Overloads& X) {<br>
+ static_cast<Overloads&&>(X).<br>
+}<br>
+<br>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:225:7 %s -o - | FileCheck -check-prefix=CHECK-LVALUE %s \<br>
+// RUN: --implicit-check-not="[#int#]ConstOverload(" \<br>
+// RUN: --implicit-check-not="[#double#]RefOverload(" \<br>
+// RUN: --implicit-check-not="[#char#]RefOverload("<br>
+// CHECK-LVALUE-DAG: [#double#]ConstOverload(<br>
+// CHECK-LVALUE-DAG: [#int#]RefOverload(<br>
+<br>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:228:12 %s -o - | FileCheck -check-prefix=CHECK-CONSTLVALUE %s \<br>
+// RUN: --implicit-check-not="[#double#]ConstOverload(" \<br>
+// RUN: --implicit-check-not="[#int#]RefOverload(" \<br>
+// RUN: --implicit-check-not="[#char#]RefOverload("<br>
+// CHECK-CONSTLVALUE: [#int#]ConstOverload(<br>
+// CHECK-CONSTLVALUE: [#double#]RefOverload(<br>
+<br>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:231:15 %s -o - | FileCheck -check-prefix=CHECK-PRVALUE %s \<br>
+// RUN: --implicit-check-not="[#int#]ConstOverload(" \<br>
+// RUN: --implicit-check-not="[#int#]RefOverload(" \<br>
+// RUN: --implicit-check-not="[#double#]RefOverload("<br>
+// CHECK-PRVALUE: [#double#]ConstOverload(<br>
+// CHECK-PRVALUE: [#char#]RefOverload(<br>
+<br>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:234:31 %s -o - | FileCheck -check-prefix=CHECK-XVALUE %s \<br>
+// RUN: --implicit-check-not="[#int#]ConstOverload(" \<br>
+// RUN: --implicit-check-not="[#int#]RefOverload(" \<br>
+// RUN: --implicit-check-not="[#double#]RefOverload("<br>
+// CHECK-XVALUE: [#double#]ConstOverload(<br>
+// CHECK-XVALUE: [#char#]RefOverload(<br>
+<br>
+void testOverloadOperator() {<br>
+ struct S {<br>
+ char operator=(int) const;<br>
+ int operator=(int);<br>
+ } s;<br>
+ return s.<br>
+}<br>
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:270:12 %s -o - | FileCheck -check-prefix=CHECK-OPER %s \<br>
+// RUN: --implicit-check-not="[#char#]operator=("<br>
+// CHECK-OPER: [#int#]operator=(<br>
+<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>