From ecb70d1e9182f70e044f037e5e225c96dd06ef8f Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryukov@google.com>
Date: Fri, 7 Dec 2018 13:17:52 +0000
Subject: [PATCH] [CodeComplete] Fix assertion failure

Summary:
...that fires when running completion inside an argument of
UnresolvedMemberExpr (see the added test).

The assertion that fires is from Sema::TryObjectArgumentInitialization:

    assert(FromClassification.isLValue());

This happens because Sema::AddFunctionCandidates does not account for
object types which are pointers. It ends up classifying them incorrectly.
All usages of the function outside code completion are used to run
overload resolution for operators. In those cases the object type being
passed is always a non-pointer type, so it's not surprising the function
did not expect a pointer in the object argument.

However, code completion reuses the same function and calls it with the
object argument coming from UnresolvedMemberExpr, which can be a pointer
if the member expr is an arrow ('->') access.

Extending AddFunctionCandidates to allow pointer object types does not
seem too crazy since all the functions down the call chain can properly
handle pointer object types if we properly classify the object argument
as an l-value, i.e. the classification of the implicitly dereferenced
pointer.

Reviewers: kadircet

Reviewed By: kadircet

Subscribers: cfe-commits

Differential Revision: https://reviews.llvm.org/D55331

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@348590 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Sema/SemaOverload.cpp                |  7 ++++++-
 test/CodeCompletion/signatures-crash.cpp | 15 +++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)
 create mode 100644 test/CodeCompletion/signatures-crash.cpp

diff --git a/lib/Sema/SemaOverload.cpp b/lib/Sema/SemaOverload.cpp
index 614b83d524..928a982e1f 100644
--- a/lib/Sema/SemaOverload.cpp
+++ b/lib/Sema/SemaOverload.cpp
@@ -6429,7 +6429,12 @@ void Sema::AddFunctionCandidates(const UnresolvedSetImpl &Fns,
         if (Expr *E = Args[0]) {
           // Use the explicit base to restrict the lookup:
           ObjectType = E->getType();
-          ObjectClassification = E->Classify(Context);
+          // Pointers in the object arguments are implicitly dereferenced, so we
+          // always classify them as l-values.
+          if (!ObjectType.isNull() && ObjectType->isPointerType())
+            ObjectClassification = Expr::Classification::makeSimpleLValue();
+          else
+            ObjectClassification = E->Classify(Context);
         } // .. else there is an implicit base.
         FunctionArgs = Args.slice(1);
       }
diff --git a/test/CodeCompletion/signatures-crash.cpp b/test/CodeCompletion/signatures-crash.cpp
new file mode 100644
index 0000000000..c58ae0cc2f
--- /dev/null
+++ b/test/CodeCompletion/signatures-crash.cpp
@@ -0,0 +1,15 @@
+struct map {
+  void find(int);
+  void find();
+};
+
+int main() {
+  map *m;
+  m->find(10);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:8:11 %s -o - | FileCheck %s
+  // CHECK: OVERLOAD: [#void#]find(<#int#>)
+
+  // Also check when the lhs is an explicit pr-value.
+  (m+0)->find(10);
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:13:15 %s -o - | FileCheck %s
+}
-- 
2.40.0