[cxx-interop] Import using decls that expose methods from private base classes

If a C++ type `Derived` inherits from `Base` privately, the public methods from `Base` should not be callable on an instance of `Derived`. However, C++ supports exposing such methods via a using declaration: `using MyPrivateBase::myPublicMethod;`.

MSVC started using this feature for `std::optional` which means Swift doesn't correctly import `var pointee: Pointee` for instantiations of `std::optional` on Windows. This prevents the automatic conformance to `CxxOptional` from being synthesized.

 rdar://114282353 / resolves #68068

Author: egorzhdan

lib/ClangImporter/ImportDecl.cpp

@@ -3237,6 +3237,58 @@ namespace {
                                 llvm::None);
     }
 
+    bool postProcessFuncDecl(FuncDecl *func, ImportedName importedName,
+                             bool importFuncWithoutSignature) {
+      auto dc = func->getDeclContext();
+
+      if (importedName.isSubscriptAccessor() && !importFuncWithoutSignature) {
+        assert(func->getParameters()->size() == 1);
+        auto typeDecl = dc->getSelfNominalTypeDecl();
+        auto parameter = func->getParameters()->get(0);
+        auto parameterType = parameter->getTypeInContext();
+        if (!typeDecl || !parameterType)
+          return false;
+        if (parameter->isInOut())
+          // Subscripts with inout parameters are not allowed in Swift.
+          return false;
+
+        auto &getterAndSetter = Impl.cxxSubscripts[{typeDecl, parameterType}];
+
+        switch (importedName.getAccessorKind()) {
+        case ImportedAccessorKind::SubscriptGetter:
+          getterAndSetter.first = func;
+          break;
+        case ImportedAccessorKind::SubscriptSetter:
+          getterAndSetter.second = func;
+          break;
+        default:
+          llvm_unreachable("invalid subscript kind");
+        }
+
+        Impl.markUnavailable(func, "use subscript");
+      }
+
+      if (importedName.isDereferenceAccessor() && !importFuncWithoutSignature) {
+        auto typeDecl = dc->getSelfNominalTypeDecl();
+        auto &getterAndSetter = Impl.cxxDereferenceOperators[typeDecl];
+
+        switch (importedName.getAccessorKind()) {
+        case ImportedAccessorKind::DereferenceGetter:
+          getterAndSetter.first = func;
+          break;
+        case ImportedAccessorKind::DereferenceSetter:
+          getterAndSetter.second = func;
+          break;
+        default:
+          llvm_unreachable("invalid dereference operator kind");
+        }
+
+        Impl.markUnavailable(func, "use .pointee property");
+      }
+
+      return true;
+    }
+
     Decl *importFunctionDecl(
         const clang::FunctionDecl *decl, ImportedName importedName,
         llvm::Optional<ImportedName> correctSwiftName,
@@ -3556,62 +3608,13 @@ namespace {
             func->setImportAsStaticMember();
           }
         }
+        // Someday, maybe this will need to be 'open' for C++ virtual methods.
+        func->setAccess(AccessLevel::Public);
 
-        bool makePrivate = false;
-
-        if (importedName.isSubscriptAccessor() && !importFuncWithoutSignature) {
-          assert(func->getParameters()->size() == 1);
-          auto typeDecl = dc->getSelfNominalTypeDecl();
-          auto parameter = func->getParameters()->get(0);
-          auto parameterType = parameter->getTypeInContext();
-          if (!typeDecl || !parameterType)
-            return nullptr;
-          if (parameter->isInOut())
-            // Subscripts with inout parameters are not allowed in Swift.
-            return nullptr;
-
-          auto &getterAndSetter = Impl.cxxSubscripts[{ typeDecl,
-                                                       parameterType }];
-
-          switch (importedName.getAccessorKind()) {
-          case ImportedAccessorKind::SubscriptGetter:
-            getterAndSetter.first = func;
-            break;
-          case ImportedAccessorKind::SubscriptSetter:
-            getterAndSetter.second = func;
-            break;
-          default:
-            llvm_unreachable("invalid subscript kind");
-          }
-
-          Impl.markUnavailable(func, "use subscript");
-        }
-
-        if (importedName.isDereferenceAccessor() &&
-            !importFuncWithoutSignature) {
-          auto typeDecl = dc->getSelfNominalTypeDecl();
-          auto &getterAndSetter = Impl.cxxDereferenceOperators[typeDecl];
-
-          switch (importedName.getAccessorKind()) {
-          case ImportedAccessorKind::DereferenceGetter:
-            getterAndSetter.first = func;
-            break;
-          case ImportedAccessorKind::DereferenceSetter:
-            getterAndSetter.second = func;
-            break;
-          default:
-            llvm_unreachable("invalid dereference operator kind");
-          }
-
-          Impl.markUnavailable(func, "use .pointee property");
-          makePrivate = true;
-        }
-
-        if (makePrivate)
-          func->setAccess(AccessLevel::Private);
-        else
-          // Someday, maybe this will need to be 'open' for C++ virtual methods.
-          func->setAccess(AccessLevel::Public);
+        bool success =
+            postProcessFuncDecl(func, importedName, importFuncWithoutSignature);
+        if (!success)
+          return nullptr;
       }
 
       result->setIsObjC(false);
@@ -3924,13 +3927,16 @@ namespace {
     }
 
     Decl *VisitUsingDecl(const clang::UsingDecl *decl) {
-      // Using declarations are not imported.
+      // See VisitUsingShadowDecl below.
       return nullptr;
     }
 
     Decl *VisitUsingShadowDecl(const clang::UsingShadowDecl *decl) {
-      // Only import types for now.
-      if (!isa<clang::TypeDecl>(decl->getUnderlyingDecl()))
+      // Only import:
+      //   1. Types
+      //   2. C++ methods from privately inherited base classes
+      if (!isa<clang::TypeDecl>(decl->getTargetDecl()) &&
+          !isa<clang::CXXMethodDecl>(decl->getTargetDecl()))
         return nullptr;
 
       ImportedName importedName;
@@ -3946,30 +3952,69 @@ namespace {
         return importCompatibilityTypeAlias(decl, importedName,
                                             *correctSwiftName);
 
-      auto DC =
+      auto importedDC =
           Impl.importDeclContextOf(decl, importedName.getEffectiveContext());
-      if (!DC)
+      if (!importedDC)
         return nullptr;
 
-      Decl *SwiftDecl = Impl.importDecl(decl->getUnderlyingDecl(), getActiveSwiftVersion());
-      if (!SwiftDecl)
-        return nullptr;
+      if (isa<clang::TypeDecl>(decl->getTargetDecl())) {
+        Decl *SwiftDecl = Impl.importDecl(decl->getUnderlyingDecl(), getActiveSwiftVersion());
+        if (!SwiftDecl)
+          return nullptr;
 
-      const TypeDecl *SwiftTypeDecl = dyn_cast<TypeDecl>(SwiftDecl);
-      if (!SwiftTypeDecl)
-        return nullptr;
+        const TypeDecl *SwiftTypeDecl = dyn_cast<TypeDecl>(SwiftDecl);
+        if (!SwiftTypeDecl)
+          return nullptr;
 
-      auto Loc = Impl.importSourceLoc(decl->getLocation());
-      auto Result = Impl.createDeclWithClangNode<TypeAliasDecl>(
-          decl,
-          AccessLevel::Public,
-          Impl.importSourceLoc(decl->getBeginLoc()),
-          SourceLoc(), Name,
-          Loc,
-          /*genericparams*/nullptr, DC);
-      Result->setUnderlyingType(SwiftTypeDecl->getDeclaredInterfaceType());
+        auto Loc = Impl.importSourceLoc(decl->getLocation());
+        auto Result = Impl.createDeclWithClangNode<TypeAliasDecl>(
+            decl,
+            AccessLevel::Public,
+            Impl.importSourceLoc(decl->getBeginLoc()),
+            SourceLoc(), Name,
+            Loc,
+            /*genericparams*/nullptr, importedDC);
+        Result->setUnderlyingType(SwiftTypeDecl->getDeclaredInterfaceType());
+
+        return Result;
+      }
+      if (auto targetMethod =
+              dyn_cast<clang::CXXMethodDecl>(decl->getTargetDecl())) {
+        if (isa<clang::CXXConstructorDecl>(targetMethod))
+          return nullptr;
 
-      return Result;
+        auto dc = dyn_cast<clang::CXXRecordDecl>(decl->getDeclContext());
+
+        auto targetDC = targetMethod->getDeclContext();
+        auto targetRecord = dyn_cast<clang::CXXRecordDecl>(targetDC);
+        if (!targetRecord)
+          return nullptr;
+
+        // If this struct is not inherited from the struct where the method is
+        // defined, bail.
+        if (!dc->isDerivedFrom(targetRecord))
+          return nullptr;
+
+        auto importedBaseMethod = dyn_cast_or_null<FuncDecl>(
+            Impl.importDecl(targetMethod, getActiveSwiftVersion()));
+        // This will be nullptr for a protected method of base class that is
+        // made public with a using declaration in a derived class. This is
+        // valid in C++ but we do not import such using declarations now.
+        // TODO: make this work for protected base methods.
+        if (!importedBaseMethod)
+          return nullptr;
+        auto clonedMethod = dyn_cast_or_null<FuncDecl>(
+            Impl.importBaseMemberDecl(importedBaseMethod, importedDC));
+        if (!clonedMethod)
+          return nullptr;
+
+        bool success = postProcessFuncDecl(clonedMethod, importedName, false);
+        if (!success)
+          return nullptr;
+
+        return clonedMethod;
+      }
+      return nullptr;
     }
 
     /// Add an @objc(name) attribute with the given, optional name expressed as
@@ -8353,11 +8398,17 @@ ClangImporter::Implementation::importDeclImpl(const clang::NamedDecl *ClangDecl,
   if (Result &&
       (!Result->getDeclContext()->isModuleScopeContext() ||
        isa<ClangModuleUnit>(Result->getDeclContext()))) {
+    // For using declarations that expose a method of a base class, the Clang
+    // decl is synthesized lazily when the method is actually used from Swift.
+    bool hasSynthesizedClangNode =
+        isa<clang::UsingShadowDecl>(ClangDecl) && isa<FuncDecl>(Result);
+
     // Either the Swift declaration was from stdlib,
     // or we imported the underlying decl of the typedef,
     // or we imported the decl itself.
     bool ImportedCorrectly =
         !Result->getClangDecl() || SkippedOverTypedef ||
+        hasSynthesizedClangNode ||
         Result->getClangDecl()->getCanonicalDecl() == Canon;
 
     // Or the other type is a typedef,
@@ -8380,7 +8431,7 @@ ClangImporter::Implementation::importDeclImpl(const clang::NamedDecl *ClangDecl,
       }
       assert(ImportedCorrectly);
     }
-    assert(Result->hasClangNode());
+    assert(Result->hasClangNode() || hasSynthesizedClangNode);
   }
 #else
   (void)SkippedOverTypedef;

lib/ClangImporter/ImportName.cpp

@@ -1522,6 +1522,17 @@ ImportedName NameImporter::importNameImpl(const clang::NamedDecl *D,
     return ImportedName();
   result.effectiveContext = effectiveCtx;
 
+  // If this is a using declaration, import the name of the shadowed decl and
+  // adjust the context.
+  if (auto usingShadowDecl = dyn_cast<clang::UsingShadowDecl>(D)) {
+    auto targetDecl = usingShadowDecl->getTargetDecl();
+    if (isa<clang::CXXMethodDecl>(targetDecl)) {
+      ImportedName baseName = importName(targetDecl, version, givenName);
+      baseName.effectiveContext = effectiveCtx;
+      return baseName;
+    }
+  }
+
   // Gather information from the swift_async attribute, if there is one.
   llvm::Optional<unsigned> completionHandlerParamIndex;
   bool completionHandlerFlagIsZeroOnError = false;

lib/ClangImporter/ImporterImpl.h

@@ -663,9 +663,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
   llvm::DenseMap<std::pair<ValueDecl *, DeclContext *>, ValueDecl *>
       clonedBaseMembers;
 
+public:
   ValueDecl *importBaseMemberDecl(ValueDecl *decl, DeclContext *newContext);
 
-public:
   static size_t getImportedBaseMemberDeclArity(const ValueDecl *valueDecl);
 
   // Cache for already-specialized function templates and any thunks they may

lib/ClangImporter/SwiftLookupTable.cpp

@@ -1991,6 +1991,12 @@ void importer::addEntryToLookupTable(SwiftLookupTable &table,
       }
     }
   }
+  if (auto usingDecl = dyn_cast<clang::UsingDecl>(named)) {
+    for (auto usingShadowDecl : usingDecl->shadows()) {
+      if (isa<clang::CXXMethodDecl>(usingShadowDecl->getTargetDecl()))
+        addEntryToLookupTable(table, usingShadowDecl, nameImporter);
+    }
+  }
 }
 
 /// Returns the nearest parent of \p module that is marked \c explicit in its

test/Interop/Cxx/class/inheritance/Inputs/module.modulemap

@@ -28,6 +28,11 @@ module TypeAliases {
   header "type-aliases.h"
 }
 
+module UsingBaseMembers {
+  header "using-base-members.h"
+  requires cplusplus
+}
+
 module VirtualMethods {
   header "virtual-methods.h"
   requires cplusplus

test/Interop/Cxx/class/inheritance/Inputs/using-base-members.h

@@ -0,0 +1,24 @@
+struct PublicBase {
+private:
+  int value;
+
+public:
+  int publicGetter() const { return 123; }
+  void publicSetter(int v) { value = v; }
+  void notExposed() const {}
+};
+
+struct PublicBasePrivateInheritance : private PublicBase {
+  using PublicBase::publicGetter;
+  using PublicBase::publicSetter;
+};
+
+// TODO: make this work for protected base methods as well
+//struct ProtectedBase {
+//protected:
+//  int protectedGetter() const { return 456; }
+//};
+//
+//struct ProtectedMemberPrivateInheritance : private ProtectedBase {
+//  using ProtectedBase::protectedGetter;
+//};

test/Interop/Cxx/class/inheritance/using-base-members-typechecker.swift

@@ -0,0 +1,7 @@
+// RUN: %target-typecheck-verify-swift -verify-ignore-unknown -I %S/Inputs -enable-experimental-cxx-interop
+
+import UsingBaseMembers
+
+let a = PublicBasePrivateInheritance()
+let _ = a.publicGetter()
+a.notExposed() // expected-error {{value of type 'PublicBasePrivateInheritance' has no member 'notExposed'}}

test/Interop/Cxx/class/inheritance/using-base-members.swift

@@ -0,0 +1,17 @@
+// RUN: %target-run-simple-swift(-I %S/Inputs/ -Xfrontend -enable-experimental-cxx-interop)
+//
+// REQUIRES: executable_test
+
+import StdlibUnittest
+import UsingBaseMembers
+
+var UsingBaseTestSuite = TestSuite("Using Base Members")
+
+UsingBaseTestSuite.test("PublicBasePrivateInheritance") {
+  let p = PublicBasePrivateInheritance()
+  expectEqual(123, p.publicGetter())
+  p.publicSetter(456)
+  expectEqual(456, p.publicGetter())
+}
+
+runAllTests()

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

@@ -418,4 +418,13 @@ struct DerivedFromConstIterator : public ConstIterator {};
 
 struct DerivedFromConstIteratorPrivately : private ConstIterator {};
 
+struct DerivedFromConstIteratorPrivatelyWithUsingDecl : private ConstIterator {
+  using ConstIterator::operator*;
+};
+
+struct DerivedFromAmbiguousOperatorStarPrivatelyWithUsingDecl
+    : private AmbiguousOperatorStar {
+  using AmbiguousOperatorStar::operator*;
+};
+
 #endif