Revert "[CodeComplete] Improve overload handling for C++ qualified and ref-qualified methods."

This reverts commit r362924, which causes a double-free of ShadowMapEntry.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@362944 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Sema/SemaCodeComplete.cpp

@@ -16,9 +16,7 @@
 #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"
@@ -107,7 +105,7 @@ private:
           DeclIndexPair(ND, Index));
     }
 
-    ~ShadowMapEntry() {
+    void Destroy() {
       if (DeclIndexPairVector *Vec =
               DeclOrVector.dyn_cast<DeclIndexPairVector *>()) {
         delete Vec;
@@ -154,16 +152,9 @@ 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;
@@ -239,9 +230,8 @@ 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, ExprValueKind Kind) {
+  void setObjectTypeQualifiers(Qualifiers Quals) {
     ObjectTypeQualifiers = Quals;
-    ObjectKind = Kind;
     HasObjectTypeQualifiers = true;
   }
 
@@ -1167,53 +1157,6 @@ static void setInBaseClass(ResultBuilder::Result &R) {
   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) {
@@ -1290,44 +1233,6 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
           // 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.
@@ -1348,6 +1253,11 @@ void ResultBuilder::EnterNewScope() { ShadowMaps.emplace_back(); }
 
 /// 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();
 }
 
@@ -4087,8 +3997,7 @@ void Sema::CodeCompleteOrdinaryName(Scope *S,
   // the member function to filter/prioritize the results list.
   auto ThisType = getCurrentThisType();
   if (!ThisType.isNull())
-    Results.setObjectTypeQualifiers(ThisType->getPointeeType().getQualifiers(),
-                                    VK_LValue);
+    Results.setObjectTypeQualifiers(ThisType->getPointeeType().getQualifiers());
 
   CodeCompletionDeclConsumer Consumer(Results, CurContext);
   LookupVisibleDecls(S, LookupOrdinaryName, Consumer,
@@ -4642,12 +4551,13 @@ AddObjCProperties(const CodeCompletionContext &CCContext,
   }
 }
 
-static void AddRecordMembersCompletionResults(
-    Sema &SemaRef, ResultBuilder &Results, Scope *S, QualType BaseType,
-    ExprValueKind BaseKind, RecordDecl *RD, Optional<FixItHint> AccessOpFixIt) {
+static void
+AddRecordMembersCompletionResults(Sema &SemaRef, ResultBuilder &Results,
+                                  Scope *S, QualType BaseType, 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(), BaseKind);
+  Results.setObjectTypeQualifiers(BaseType.getQualifiers());
 
   // Access to a C/C++ class, struct, or union.
   Results.allowNestedNameSpecifiers();
@@ -4728,20 +4638,18 @@ void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
     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();
-        BaseKind = VK_LValue;
-      } else if (BaseType->isObjCObjectPointerType())
+      else if (BaseType->isObjCObjectPointerType())
         /*Do nothing*/;
       else
         return false;
     }
 
     if (const RecordType *Record = BaseType->getAs<RecordType>()) {
-      AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind,
+      AddRecordMembersCompletionResults(*this, Results, S, BaseType,
                                         Record->getDecl(),
                                         std::move(AccessOpFixIt));
     } else if (const auto *TST =
@@ -4750,13 +4658,13 @@ void Sema::CodeCompleteMemberReferenceExpr(Scope *S, Expr *Base,
       if (const auto *TD =
               dyn_cast_or_null<ClassTemplateDecl>(TN.getAsTemplateDecl())) {
         CXXRecordDecl *RD = TD->getTemplatedDecl();
-        AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind,
-                                          RD, std::move(AccessOpFixIt));
+        AddRecordMembersCompletionResults(*this, Results, S, BaseType, RD,
+                                          std::move(AccessOpFixIt));
       }
     } else if (const auto *ICNT = BaseType->getAs<InjectedClassNameType>()) {
       if (auto *RD = ICNT->getDecl())
-        AddRecordMembersCompletionResults(*this, Results, S, BaseType, BaseKind,
-                                          RD, std::move(AccessOpFixIt));
+        AddRecordMembersCompletionResults(*this, Results, S, BaseType, RD,
+                                          std::move(AccessOpFixIt));
     } else if (!IsArrow && BaseType->isObjCObjectPointerType()) {
       // Objective-C property reference.
       AddedPropertiesSet AddedProperties;

test/CodeCompletion/member-access.cpp

@@ -210,66 +210,3 @@ 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=(
-