Merge pull request #63653 from apple/egorzhdan/cxx-ambiguous-pointee

[cxx-interop] Do not synthesize ambiguous pointee properties

Author: egorzhdan

lib/ClangImporter/ImportDecl.cpp

@@ -2127,6 +2127,17 @@ namespace {
       // The name of every member.
       llvm::DenseSet<StringRef> allMemberNames;
 
+      bool hasConstOperatorStar = false;
+      for (auto member : decl->decls()) {
+        if (auto method = dyn_cast<clang::CXXMethodDecl>(member)) {
+          if (method->getOverloadedOperator() ==
+                  clang::OverloadedOperatorKind::OO_Star &&
+              method->param_empty() && method->isConst())
+            hasConstOperatorStar = true;
+        }
+      }
+      bool hasSynthesizedPointeeProperty = false;
+
       // FIXME: Import anonymous union fields and support field access when
       // it is nested in a struct.
       for (auto m : decl->decls()) {
@@ -2210,9 +2221,16 @@ namespace {
             if (cxxOperatorKind == clang::OO_Star && cxxMethod->param_empty()) {
               // This is a dereference operator. We synthesize a computed
               // property called `pointee` for it.
-              VarDecl *pointeeProperty =
-                  synthesizer.makeDereferencedPointeeProperty(MD);
-              result->addMember(pointeeProperty);
+
+              // If this record has multiple overloads of `operator*`, prefer
+              // the const overload if it exists.
+              if ((cxxMethod->isConst() || !hasConstOperatorStar) &&
+                  !hasSynthesizedPointeeProperty) {
+                VarDecl *pointeeProperty =
+                    synthesizer.makeDereferencedPointeeProperty(MD);
+                result->addMember(pointeeProperty);
+                hasSynthesizedPointeeProperty = true;
+              }
 
               Impl.markUnavailable(MD, "use .pointee property");
               MD->overwriteAccess(AccessLevel::Private);

test/Interop/Cxx/operators/Inputs/member-inline.h

@@ -394,4 +394,21 @@ struct ConstIteratorByVal {
   int operator*() const { return value; }
 };
 
+struct AmbiguousOperatorStar {
+private:
+  int value = 567;
+public:
+  int &operator*() { return value; }
+  const int &operator*() const { return value; }
+};
+
+struct AmbiguousOperatorStar2 {
+private:
+  int value = 678;
+public:
+  int &operator*() & { return value; }
+  const int &operator*() const & { return value; }
+  const int &&operator*() const && { return static_cast<const int &&>(value); }
+};
+
 #endif

test/Interop/Cxx/operators/member-inline-module-interface.swift

@@ -221,3 +221,23 @@
 // CHECK:   @available(*, unavailable, message: "use .pointee property")
 // CHECK:   func __operatorStar() -> Int32
 // CHECK: }
+
+// CHECK: struct AmbiguousOperatorStar {
+// CHECK-NEXT:   var pointee: Int32 { get }
+// CHECK-NEXT:   init()
+// CHECK-NEXT:   @available(*, unavailable, message: "use .pointee property")
+// CHECK-NEXT:   mutating func __operatorStar() -> UnsafeMutablePointer<Int32>
+// CHECK-NEXT:   @available(*, unavailable, message: "use .pointee property")
+// CHECK-NEXT:   func __operatorStar() -> UnsafePointer<Int32>
+// CHECK-NEXT: }
+
+// CHECK: struct AmbiguousOperatorStar2 {
+// CHECK-NEXT:   var pointee: Int32 { get }
+// CHECK-NEXT:   init()
+// CHECK-NEXT:   @available(*, unavailable, message: "use .pointee property")
+// CHECK-NEXT:   mutating func __operatorStar() -> UnsafeMutablePointer<Int32>
+// CHECK-NEXT:   @available(*, unavailable, message: "use .pointee property")
+// CHECK-NEXT:   func __operatorStar() -> UnsafePointer<Int32>
+// CHECK-NEXT:   @available(*, unavailable, message: "use .pointee property")
+// CHECK-NEXT:   func __operatorStar() -> UnsafePointer<Int32>
+// CHECK-NEXT: }

test/Interop/Cxx/operators/member-inline.swift

@@ -363,4 +363,16 @@ OperatorsTestSuite.test("ConstIteratorByVal.pointee") {
   expectEqual(456, res)
 }
 
+OperatorsTestSuite.test("AmbiguousOperatorStar.pointee") {
+  let stars = AmbiguousOperatorStar()
+  let res = stars.pointee
+  expectEqual(567, res)
+}
+
+OperatorsTestSuite.test("AmbiguousOperatorStar2.pointee") {
+  let stars = AmbiguousOperatorStar2()
+  let res = stars.pointee
+  expectEqual(678, res)
+}
+
 runAllTests()

test/Interop/Cxx/stdlib/Inputs/module.modulemap

@@ -13,6 +13,11 @@ module StdMap {
   requires cplusplus
 }
 
+module StdOptional {
+  header "std-optional.h"
+  requires cplusplus
+}
+
 module StdSet {
   header "std-set.h"
   requires cplusplus

test/Interop/Cxx/stdlib/Inputs/std-optional.h

@@ -0,0 +1,12 @@
+#ifndef TEST_INTEROP_CXX_STDLIB_INPUTS_STD_OPTIONAL_H
+#define TEST_INTEROP_CXX_STDLIB_INPUTS_STD_OPTIONAL_H
+
+#include <optional>
+
+using CxxOptional = std::optional<int>;
+
+inline CxxOptional getNonNilOptional() { return {123}; }
+
+inline CxxOptional getNilOptional() { return {std::nullopt}; }
+
+#endif // TEST_INTEROP_CXX_STDLIB_INPUTS_STD_OPTIONAL_H

test/Interop/Cxx/stdlib/use-std-optional.swift

@@ -0,0 +1,18 @@
+// RUN: %target-run-simple-swift(-I %S/Inputs -Xfrontend -enable-experimental-cxx-interop -Xcc -std=c++17)
+//
+// REQUIRES: executable_test
+// REQUIRES: OS=macosx
+
+import StdlibUnittest
+import StdOptional
+import CxxStdlib
+
+var StdOptionalTestSuite = TestSuite("StdOptional")
+
+StdOptionalTestSuite.test("pointee") {
+  let nonNilOpt = getNonNilOptional()
+  let pointee = nonNilOpt.pointee
+  expectEqual(123, pointee)
+}
+
+runAllTests()