Fix tests and re-enable support for CXX operators

Author: Huddie

lib/ClangImporter/ClangImporter.cpp

@@ -2807,7 +2807,6 @@ static bool isVisibleFromModule(const ClangModuleUnit *ModuleFilter,
 
   const clang::Decl *D = ClangNode.castAsDecl();
   auto &ClangASTContext = ModuleFilter->getClangASTContext();
-
   // We don't handle Clang submodules; pop everything up to the top-level
   // module.
   auto OwningClangModule = getClangTopLevelOwningModule(ClangNode,
@@ -2880,8 +2879,8 @@ class FilteringVisibleDeclConsumer : public swift::VisibleDeclConsumer {
 
   void foundDecl(ValueDecl *VD, DeclVisibilityKind Reason,
                  DynamicLookupInfo dynamicLookupInfo) override {
-    if (isVisibleFromModule(ModuleFilter, VD))
-      NextConsumer.foundDecl(VD, Reason, dynamicLookupInfo);
+    if (!VD->hasClangNode() || isVisibleFromModule(ModuleFilter, VD))
+        NextConsumer.foundDecl(VD, Reason, dynamicLookupInfo);
   }
 };
 
@@ -3112,7 +3111,7 @@ void ClangImporter::lookupValue(DeclName name, VisibleDeclConsumer &consumer) {
 ClangNode ClangImporter::getEffectiveClangNode(const Decl *decl) const {
   // Directly...
   if (auto clangNode = decl->getClangNode())
-    return clangNode;
+      return clangNode;
 
   // Or via the nested "Code" enum.
   if (auto *errorWrapper = dyn_cast<StructDecl>(decl)) {
@@ -4121,43 +4120,43 @@ bool ClangImporter::Implementation::lookupValue(SwiftLookupTable &table,
   auto clangTU = clangCtx.getTranslationUnitDecl();
 
   bool declFound = false;
-  // For operators we have to look up static member functions in addition to the
-  // top-level function lookup below.
-  auto declBaseName = (SwiftContext.LangOpts.EnableCXXInterop && name.isOperator())
-          ? DeclBaseName(SwiftContext.getIdentifier("__operator" + getOperatorNameForToken(std::string{name.getBaseName().getIdentifier()})))
-          : name.getBaseName();
 
   if (name.isOperator()) {
-    for (auto entry : table.lookupMemberOperators(declBaseName)) {
-        if (isVisibleClangEntry(entry)) {
-            if (auto decl = dyn_cast_or_null<ValueDecl>(
-                    importDeclReal(entry->getMostRecentDecl(), CurrentVersion))) {
-                consumer.foundDecl(decl, DeclVisibilityKind::VisibleAtTopLevel);
-                declFound = true;
-                for (auto alternate: getAlternateDecls(decl)) {
-                    if (alternate->getName().matchesRef(name)) {
-                        consumer.foundDecl(alternate, DeclVisibilityKind::DynamicLookup,
-                                           DynamicLookupInfo::AnyObject);
-                    }
-                }
-            }
-        }
-    }
-        for (auto entry : table.lookupMemberOperators(name.getBaseName())) {
-            if (isVisibleClangEntry(entry)) {
-                if (auto decl = dyn_cast_or_null<ValueDecl>(
-                        importDeclReal(entry->getMostRecentDecl(), CurrentVersion))) {
-                    consumer.foundDecl(decl, DeclVisibilityKind::VisibleAtTopLevel);
-                    declFound = true;
-                    for (auto alternate : getAlternateDecls(decl)) {
-                        if (alternate->getName().matchesRef(name)) {
-                            consumer.foundDecl(alternate, DeclVisibilityKind::DynamicLookup,
-                                               DynamicLookupInfo::AnyObject);
-                        }
-                    }
-                }
-            }
-    }
+      for (auto entry : table.lookupMemberOperators(name.getBaseName())) {
+          if (isVisibleClangEntry(entry)) {
+              if (auto decl = dyn_cast_or_null<ValueDecl>(
+                      importDeclReal(entry->getMostRecentDecl(), CurrentVersion))) {
+                  consumer.foundDecl(decl, DeclVisibilityKind::VisibleAtTopLevel);
+                  declFound = true;
+                  for (auto alternate : getAlternateDecls(decl)) {
+                      if (alternate->getName().matchesRef(name)) {
+                          consumer.foundDecl(alternate, DeclVisibilityKind::DynamicLookup,
+                                             DynamicLookupInfo::AnyObject);
+                      }
+                  }
+              }
+          }
+      }
+
+      // If CXXInterop is enabled we need to check the modified operator name as well
+      if(SwiftContext.LangOpts.EnableCXXInterop) {
+          auto declBaseName = DeclBaseName(SwiftContext.getIdentifier("__operator" + getOperatorNameForToken(name.getBaseName().getIdentifier().str().str())));
+          for (auto entry : table.lookupMemberOperators(declBaseName)) {
+              if (isVisibleClangEntry(entry)) {
+                  if (auto decl = dyn_cast_or_null<ValueDecl>(
+                          importDeclReal(entry->getMostRecentDecl(), CurrentVersion))) {
+                      consumer.foundDecl(decl, DeclVisibilityKind::VisibleAtTopLevel);
+                      declFound = true;
+                      for (auto alternate: getAlternateDecls(decl)) {
+                          if (alternate->getName().matchesRef(name)) {
+                              consumer.foundDecl(alternate, DeclVisibilityKind::DynamicLookup,
+                                                 DynamicLookupInfo::AnyObject);
+                          }
+                      }
+                  }
+              }
+          }
+      }
   }
 
   for (auto entry : table.lookup(name.getBaseName(), clangTU)) {

lib/ClangImporter/ImportDecl.cpp

@@ -3601,12 +3601,12 @@ namespace {
                 cxxOperatorKind != clang::OverloadedOperatorKind::OO_Call &&
                 cxxOperatorKind != clang::OverloadedOperatorKind::OO_Subscript) {
 
-              auto d = makeOperator(MD, cxxMethod);
-              result->addMember(d);
+              auto opFuncDecl = makeOperator(MD, cxxMethod);
 
-              Impl.addAlternateDecl(MD, d);
+              Impl.addAlternateDecl(MD, opFuncDecl);
+
+              Impl.markUnavailable(MD, "use " + std::string{clang::getOperatorSpelling(cxxOperatorKind)} + " instead");
 
-              Impl.markUnavailable(MD, "use - operator");
               // Make the actual member operator private.
               MD->overwriteAccess(AccessLevel::Private);
             }
@@ -4151,11 +4151,6 @@ namespace {
       if (!dc)
         return nullptr;
 
-      // Support for importing operators is temporarily disabled: rdar://91070109
-      if (decl->getDeclName().getNameKind() == clang::DeclarationName::CXXOperatorName &&
-          decl->getDeclName().getCXXOverloadedOperator() != clang::OO_Subscript)
-        return nullptr;
-
       // Handle cases where 2 CXX methods differ strictly in "constness"
       // In such a case append a suffix ("Mutating") to the mutable version
       // of the method when importing to swift

test/Interop/Cxx/implementation-only-imports/Inputs/module.modulemap

@@ -28,4 +28,9 @@ module DeclB {
     requires cplusplus
 }
 
+module Helper {
+    header "helper.h"
+    export *
+    requires cplusplus
+}
 

test/Interop/Cxx/implementation-only-imports/check-operator-visibility-inversed.swift

@@ -8,8 +8,6 @@
 // that the operator decl can be found when at least one of the
 // modules is not `@_implementationOnly`.
 
-// XFAIL: *
-
 import UserA
 @_implementationOnly import UserB
 

test/Interop/Cxx/implementation-only-imports/check-operator-visibility.swift

@@ -8,8 +8,6 @@
 // that the operator decl can be found when at least one of the
 // modules is not `@_implementationOnly`.
 
-// XFAIL: *
-
 @_implementationOnly import UserA
 import UserB
 

test/Interop/Cxx/implementation-only-imports/skip-forward-declarations.swift

@@ -12,4 +12,4 @@ public func createAWrapper() {
   let _ = MagicWrapper()
 }
 
-// CHECK: struct 'MagicWrapper' cannot be used in an '@inlinable' function because 'UserA' was imported implementation-only
+// CHECK: struct 'MagicWrapper' cannot be used in an '@inlinable' function because 'Helper' was imported implementation-only

test/Interop/Cxx/namespace/free-functions-module-interface.swift

@@ -7,8 +7,7 @@
 // CHECK-NEXT:   struct X {
 // CHECK-NEXT:     init()
 // CHECK-NEXT:   }
-// TODO: check this again when operators are re-enabled: rdar://91070109 (also please enable this in the execution test "free-functions.swift")
-// CHECK-NOT:   static func +
+// CHECK-NEXT:   static func + (_: FunctionsNS1.X, _: FunctionsNS1.X) -> UnsafePointer<CChar>!
 // CHECK-NEXT:   static func sameNameInChild() -> UnsafePointer<CChar>!
 // CHECK-NEXT:   static func sameNameInSibling() -> UnsafePointer<CChar>!
 // CHECK-NEXT:   enum FunctionsNS2 {
@@ -21,8 +20,7 @@
 // CHECK-NEXT:   static func definedInDefs() -> UnsafePointer<CChar>!
 // CHECK-NEXT: }
 
-// TODO: check this again when operators are re-enabled: rdar://91070109 (also please enable this in the execution test "free-functions.swift")
-// CHECK-NOT: static func +
+// CHECK-NEXT: static func + (_: FunctionsNS1.X, _: FunctionsNS1.X) -> UnsafePointer<CChar>!
 
 // CHECK-NEXT: enum FunctionsNS4 {
 // CHECK-NEXT:   static func sameNameInSibling() -> UnsafePointer<CChar>!

test/Interop/Cxx/namespace/free-functions.swift

@@ -20,10 +20,9 @@ NamespacesTestSuite.test("Basic functions") {
   expectEqual(String(cString: basicFunctionLowestLevelCString!),
               "FunctionsNS1::FunctionsNS2::FunctionsNS3::basicFunctionLowestLevel")
 
-// TODO: check this again when operators are re-enabled: rdar://91070109
-//   let x = FunctionsNS1.X()
-//   expectEqual(String(cString: x + x),
-//               "FunctionsNS1::operator+(X, X)")
+  let x = FunctionsNS1.X()
+  expectEqual(String(cString: x + x),
+              "FunctionsNS1::operator+(X, X)")
 }
 
 NamespacesTestSuite.test("Forward declared functions") {

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

@@ -1,7 +1,6 @@
 // RUN: %target-swift-emit-ir %s -I %S/Inputs -enable-experimental-cxx-interop | %FileCheck %s
 //
 // We can't yet call member functions correctly on Windows (SR-13129).
-// XFAIL: *
 
 import MemberInline
 

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

@@ -1,5 +1,4 @@
 // RUN: %target-swift-ide-test -print-module -module-to-print=MemberInline -I %S/Inputs -source-filename=x -enable-experimental-cxx-interop | %FileCheck %s
-// XFAIL: *
 
 // CHECK: struct LoadableIntWrapper {
 // CHECK:   static func - (lhs: inout LoadableIntWrapper, rhs: LoadableIntWrapper) -> LoadableIntWrapper