[clang][CodeComplete] Recurse into the subexpression of deref operator in getApproximateType

[zyn0217]

The issue with the previous implementation bc31be7 was that getApproximateType could potentially return a null QualType for a dereferencing operator, which is not what its caller wants.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-tools-extra

Author: Younan Zhang (zyn0217)

Changes

The issue with the previous implementation bc31be7 was that getApproximateType could potentially return a null QualType for a dereferencing operator, which is not what its caller wants.

---

Full diff: https://github.com/llvm/llvm-project/pull/93404.diff

2 Files Affected:

• (modified) clang/lib/Sema/SemaCodeComplete.cpp (+10-2)
• (modified) clang/test/CodeCompletion/member-access.cpp (+16)

diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index 328641ed94881..73ed99e099869 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -5680,8 +5680,16 @@ QualType getApproximateType(const Expr *E) {
     }
   }
   if (const auto *UO = llvm::dyn_cast<UnaryOperator>(E)) {
-    if (UO->getOpcode() == UnaryOperatorKind::UO_Deref)
-      return UO->getSubExpr()->getType()->getPointeeType();
+    if (UO->getOpcode() == UnaryOperatorKind::UO_Deref) {
+      // We recurse into the subexpression because it could be of dependent
+      // type.
+      QualType SubType = getApproximateType(UO->getSubExpr());
+      if (auto Pointee = SubType->getPointeeType(); !Pointee.isNull())
+        return Pointee;
+      // Our caller expects a non-null result, even though the SubType is
+      // supposed to have a pointee.
+      return SubType;
+    }
   }
   return Unresolved;
 }
diff --git a/clang/test/CodeCompletion/member-access.cpp b/clang/test/CodeCompletion/member-access.cpp
index 9f8c21c0bca6d..912f269db6c1a 100644
--- a/clang/test/CodeCompletion/member-access.cpp
+++ b/clang/test/CodeCompletion/member-access.cpp
@@ -367,4 +367,20 @@ class A {
 // CHECK-DEREF-THIS: [#void#]function()
   }
 };
+
+template <typename Element>
+struct RepeatedField {
+  void Add();
+};
+
+template <typename T>
+RepeatedField<T>* MutableRepeatedField() {}
+
+template <class T>
+void Foo() {
+  auto& C = *MutableRepeatedField<T>();
+  C.
+}
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:382:5 %s -o - | FileCheck -check-prefix=CHECK-DEREF-DEPENDENT %s
+// CHECK-DEREF-DEPENDENT: [#void#]Add()
 }

[HighCommander4]

Thank you! Looks good with one suggested change.

[HighCommander4]

Perhaps falling through to the return Unresolved would be better? That's a dependent representation of the pointee type, so it seems less misleading than returning the pointer type. (And it's also what other branches in this function do.)

[zyn0217]

Thanks, I think that makes sense in the case of dereferencing a non-pointer type, which is ill-formed and we probably should offer nothing for completion.

[zyn0217]

(I'll merge it shortly if CI turns green because I encountered the bug the other day in our codebase. Apologies I didn't point it out in my review for bc31be7)