Re-land "[CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods."
ShadowMapEntry is now really, truly a normal class.
llvm-svn: 362950
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index 4575d4f..d2e7766 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -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"
@@ -82,6 +84,15 @@
public:
ShadowMapEntry() : DeclOrVector(), SingleDeclIndex(0) {}
+ ShadowMapEntry(const ShadowMapEntry &) = delete;
+ ShadowMapEntry(ShadowMapEntry &&Move) { *this = std::move(Move); }
+ ShadowMapEntry &operator=(const ShadowMapEntry &) = delete;
+ ShadowMapEntry &operator=(ShadowMapEntry &&Move) {
+ SingleDeclIndex = Move.SingleDeclIndex;
+ DeclOrVector = Move.DeclOrVector;
+ Move.DeclOrVector = nullptr;
+ return *this;
+ }
void Add(const NamedDecl *ND, unsigned Index) {
if (DeclOrVector.isNull()) {
@@ -105,7 +116,7 @@
DeclIndexPair(ND, Index));
}
- void Destroy() {
+ ~ShadowMapEntry() {
if (DeclIndexPairVector *Vec =
DeclOrVector.dyn_cast<DeclIndexPairVector *>()) {
delete Vec;
@@ -152,9 +163,16 @@
/// 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 +248,9 @@
/// 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 +1176,53 @@
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 +1299,44 @@
// 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.
@@ -1253,11 +1357,6 @@
/// Exit from the current scope.
void ResultBuilder::ExitScope() {
- for (ShadowMap::iterator E = ShadowMaps.back().begin(),
- EEnd = ShadowMaps.back().end();
- E != EEnd; ++E)
- E->second.Destroy();
-
ShadowMaps.pop_back();
}
@@ -3997,7 +4096,8 @@
// 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 +4651,12 @@
}
}
-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 +4737,20 @@
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 +4759,13 @@
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;