Merge pull request #62557 from apple/egorzhdan/cxx-lookup-op-template

[cxx-interop] Fix lookup failure for operators in templated classes

Author: egorzhdan

include/swift/ClangImporter/ClangImporter.h

@@ -275,6 +275,8 @@ class ClangImporter final : public ClangModuleLoader {
   ConcreteDeclRef getCXXFunctionTemplateSpecialization(
           SubstitutionMap subst, ValueDecl *decl) override;
 
+  FuncDecl *getCXXSynthesizedOperatorFunc(FuncDecl *decl);
+
   /// Just like Decl::getClangNode() except we look through to the 'Code'
   /// enum of an error wrapper struct.
   ClangNode getEffectiveClangNode(const Decl *decl) const;

lib/ClangImporter/ClangDerivedConformances.cpp

@@ -17,14 +17,32 @@
 #include "swift/ClangImporter/ClangImporterRequests.h"
 
 using namespace swift;
+using namespace swift::importer;
 
 /// Alternative to `NominalTypeDecl::lookupDirect`.
 /// This function does not attempt to load extensions of the nominal decl.
 static TinyPtrVector<ValueDecl *>
 lookupDirectWithoutExtensions(NominalTypeDecl *decl, Identifier id) {
-  // First see if there is a Clang decl with the given name.
-  TinyPtrVector<ValueDecl *> result = evaluateOrDefault(
-      decl->getASTContext().evaluator, ClangRecordMemberLookup({decl, id}), {});
+  ASTContext &ctx = decl->getASTContext();
+  auto *importer = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
+
+  TinyPtrVector<ValueDecl *> result;
+
+  if (id.isOperator()) {
+    auto underlyingId =
+        ctx.getIdentifier(getPrivateOperatorName(std::string(id)));
+    TinyPtrVector<ValueDecl *> underlyingFuncs = evaluateOrDefault(
+        ctx.evaluator, ClangRecordMemberLookup({decl, underlyingId}), {});
+    for (auto it : underlyingFuncs) {
+      if (auto synthesizedFunc =
+              importer->getCXXSynthesizedOperatorFunc(cast<FuncDecl>(it)))
+        result.push_back(synthesizedFunc);
+    }
+  } else {
+    // See if there is a Clang decl with the given name.
+    result = evaluateOrDefault(ctx.evaluator,
+                               ClangRecordMemberLookup({decl, id}), {});
+  }
 
   // Check if there are any synthesized Swift members that match the name.
   for (auto member : decl->getCurrentMembersWithoutLoading()) {

lib/ClangImporter/ClangImporter.cpp

@@ -88,17 +88,6 @@ using clang::CompilerInvocation;
 
 #pragma mark Internal data structures
 
-namespace {
-static std::string getOperatorNameForToken(std::string OperatorToken) {
-#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, MemberOnly)  \
-  if (OperatorToken == Spelling) {                                             \
-    return #Name;                                                              \
-  };
-#include "clang/Basic/OperatorKinds.def"
-  return "None";
-}
-} // namespace
-
 namespace {
   class HeaderImportCallbacks : public clang::PPCallbacks {
     ClangImporter::Implementation &Impl;
@@ -4058,6 +4047,8 @@ bool ClangImporter::Implementation::lookupValue(SwiftLookupTable &table,
 
   auto &clangCtx = getClangASTContext();
   auto clangTU = clangCtx.getTranslationUnitDecl();
+  auto *importer =
+      static_cast<ClangImporter *>(SwiftContext.getClangModuleLoader());
 
   bool declFound = false;
 
@@ -4075,33 +4066,17 @@ bool ClangImporter::Implementation::lookupValue(SwiftLookupTable &table,
     // If CXXInterop is enabled we need to check the modified operator name as
     // well
     if (SwiftContext.LangOpts.EnableCXXInterop) {
-      auto funcBaseName = DeclBaseName(SwiftContext.getIdentifier(
-          "__operator" + getOperatorNameForToken(
-                             name.getBaseName().getIdentifier().str().str())));
+      auto funcBaseName =
+          DeclBaseName(SwiftContext.getIdentifier(getPrivateOperatorName(
+              name.getBaseName().getIdentifier().str().str())));
       for (auto entry : table.lookupMemberOperators(funcBaseName)) {
         if (isVisibleClangEntry(entry)) {
-          if (auto func = dyn_cast_or_null<ValueDecl>(
+          if (auto func = dyn_cast_or_null<FuncDecl>(
                   importDeclReal(entry->getMostRecentDecl(), CurrentVersion))) {
-            // `func` is not an operator, it is a regular function which has a
-            // name that starts with `__operator`. We were asked for a
-            // corresponding synthesized Swift operator, so let's retrieve it.
-
-            // The synthesized Swift operator was added as an alternative decl
-            // for `func`.
-            auto alternateDecls = getAlternateDecls(func);
-            // Did we actually synthesize an operator for `func`?
-            if (alternateDecls.empty())
-              continue;
-            // If we did, then we should have only synthesized one.
-            assert(alternateDecls.size() == 1 &&
-                   "expected only the synthesized operator as an alternative");
-
-            auto synthesizedOperator = alternateDecls.front();
-            assert(synthesizedOperator->isOperator() &&
-                   "expected the alternative to be a synthesized operator");
-
-            consumer.foundDecl(synthesizedOperator,
-                               DeclVisibilityKind::VisibleAtTopLevel);
+            if (auto synthesizedOperator =
+                    importer->getCXXSynthesizedOperatorFunc(func))
+              consumer.foundDecl(synthesizedOperator,
+                                 DeclVisibilityKind::VisibleAtTopLevel);
           }
         }
       }
@@ -6095,6 +6070,27 @@ ClangImporter::getCXXFunctionTemplateSpecialization(SubstitutionMap subst,
   return ConcreteDeclRef(newDecl);
 }
 
+FuncDecl *ClangImporter::getCXXSynthesizedOperatorFunc(FuncDecl *decl) {
+  // `decl` is not an operator, it is a regular function which has a
+  // name that starts with `__operator`. We were asked for a
+  // corresponding synthesized Swift operator, so let's retrieve it.
+
+  // The synthesized Swift operator was added as an alternative decl
+  // for `func`.
+  auto alternateDecls = Impl.getAlternateDecls(decl);
+  // Did we actually synthesize an operator for `func`?
+  if (alternateDecls.empty())
+    return nullptr;
+  // If we did, then we should have only synthesized one.
+  assert(alternateDecls.size() == 1 &&
+         "expected only the synthesized operator as an alternative");
+
+  auto synthesizedOperator = alternateDecls.front();
+  assert(synthesizedOperator->isOperator() &&
+         "expected the alternative to be a synthesized operator");
+  return cast<FuncDecl>(synthesizedOperator);
+}
+
 bool ClangImporter::isCXXMethodMutating(const clang::CXXMethodDecl *method) {
   if (isa<clang::CXXConstructorDecl>(method) || !method->isConst())
     return true;

lib/ClangImporter/ImporterImpl.h

@@ -1947,6 +1947,15 @@ inline bool requiresCPlusPlus(const clang::Module *module) {
   });
 }
 
+inline std::string getPrivateOperatorName(const std::string &OperatorToken) {
+#define OVERLOADED_OPERATOR(Name, Spelling, Token, Unary, Binary, MemberOnly)  \
+  if (OperatorToken == Spelling) {                                             \
+    return "__operator" #Name;                                                 \
+  };
+#include "clang/Basic/OperatorKinds.def"
+  return "None";
+}
+
 }
 }
 

test/Interop/Cxx/stdlib/overlay/Inputs/custom-iterator.h

@@ -463,4 +463,37 @@ struct HasNoPlusEqualOperator {
   }
 };
 
+// MARK: Templated iterators
+
+template <typename T>
+struct TemplatedIterator {
+  T value;
+
+  using iterator_category = std::input_iterator_tag;
+  using value_type = T;
+  using pointer = T *;
+  using reference = const T &;
+  using difference_type = int;
+
+  TemplatedIterator(int value) : value(value) {}
+  TemplatedIterator(const TemplatedIterator &other) = default;
+
+  const int &operator*() const { return value; }
+
+  TemplatedIterator &operator++() {
+    value++;
+    return *this;
+  }
+  TemplatedIterator operator++(int) {
+    auto tmp = ConstIterator(value);
+    value++;
+    return tmp;
+  }
+  bool operator==(const TemplatedIterator &other) const {
+    return value == other.value;
+  }
+};
+
+using TemplatedIteratorInt = TemplatedIterator<int>;
+
 #endif // TEST_INTEROP_CXX_STDLIB_INPUTS_CUSTOM_ITERATOR_H

test/Interop/Cxx/stdlib/overlay/custom-iterator-module-interface.swift

@@ -85,3 +85,10 @@
 // CHECK-NOT: struct HasNoIncrementOperator : UnsafeCxxInputIterator
 // CHECK-NOT: struct HasNoPreIncrementOperator : UnsafeCxxInputIterator
 // CHECK-NOT: struct HasNoDereferenceOperator : UnsafeCxxInputIterator
+
+// CHECK: struct __CxxTemplateInst17TemplatedIteratorIiE : UnsafeCxxInputIterator {
+// CHECK:   var pointee: Int32 { get }
+// CHECK:   func successor() -> __CxxTemplateInst17TemplatedIteratorIiE
+// CHECK:   typealias Pointee = Int32
+// CHECK:   static func == (lhs: __CxxTemplateInst17TemplatedIteratorIiE, other: __CxxTemplateInst17TemplatedIteratorIiE) -> Bool
+// CHECK: }

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

@@ -5,9 +5,6 @@
 // Enable this everywhere once we have a solution for modularizing other C++ stdlibs: rdar://87654514
 // REQUIRES: OS=macosx || OS=linux-gnu
 
-// This test is failing in Linux CI currently.
-// REQUIRES: rdar102420290
-
 import StdlibUnittest
 import StdMap
 import CxxStdlib