[PrintAsCxx] Fix printing of C++ enum args

beccadax · beccadax · commit 7fa35d4d6ab6 · 2024-06-21T16:01:55.000-07:00
Because imported enums are @objc, they were treated as unsupported in C++ and therefore ineligible to be printed in a C++ generated header. Narrow this logic so that only @objc *classes* are excluded, and update related printing logic to support enums correctly. Fixes rdar://124262637.
diff --git a/lib/AST/SwiftNameTranslation.cpp b/lib/AST/SwiftNameTranslation.cpp
@@ -211,8 +211,6 @@ swift::cxx_translation::getNameForCxx(const ValueDecl *VD,
 
 swift::cxx_translation::DeclRepresentation
 swift::cxx_translation::getDeclRepresentation(const ValueDecl *VD) {
-  if (VD->isObjC())
-    return {Unsupported, UnrepresentableObjC};
   if (getActorIsolation(const_cast<ValueDecl *>(VD)).isActorIsolated())
     return {Unsupported, UnrepresentableIsolatedInActor};
   if (isa<MacroDecl>(VD))
@@ -238,6 +236,8 @@ swift::cxx_translation::getDeclRepresentation(const ValueDecl *VD) {
     // Swift's consume semantics are not yet supported in C++.
     if (!typeDecl->canBeCopyable())
       return {Unsupported, UnrepresentableMoveOnly};
+    if (isa<ClassDecl>(VD) && VD->isObjC())
+      return {Unsupported, UnrepresentableObjC};
     if (typeDecl->isGeneric()) {
       if (isa<ClassDecl>(VD))
         return {Unsupported, UnrepresentableGeneric};
diff --git a/lib/PrintAsClang/DeclAndTypePrinter.cpp b/lib/PrintAsClang/DeclAndTypePrinter.cpp
@@ -2925,16 +2925,34 @@ static bool isEnumExposableToCxx(const ValueDecl *VD,
 }
 
 bool DeclAndTypePrinter::shouldInclude(const ValueDecl *VD) {
-  return !VD->isInvalid() && (!requiresExposedAttribute || hasExposeAttr(VD)) &&
-         (outputLang == OutputLanguageMode::Cxx
-              ? cxx_translation::isVisibleToCxx(VD, minRequiredAccess) &&
-                    isExposedToThisModule(M, VD, exposedModules) &&
-                    cxx_translation::isExposableToCxx(VD) &&
-                    isEnumExposableToCxx(VD, *this)
-              : isVisibleToObjC(VD, minRequiredAccess)) &&
-         !VD->getAttrs().hasAttribute<ImplementationOnlyAttr>() &&
-         !isAsyncAlternativeOfOtherDecl(VD) &&
-         !excludeForObjCImplementation(VD);
+  if (VD->isInvalid())
+    return false;
+
+  if (requiresExposedAttribute && !hasExposeAttr(VD))
+    return false;
+
+  if (!isVisible(VD))
+    return false;
+
+  if (outputLang == OutputLanguageMode::Cxx) {
+    if (!isExposedToThisModule(M, VD, exposedModules))
+      return false;
+    if (!cxx_translation::isExposableToCxx(VD))
+      return false;
+    if (!isEnumExposableToCxx(VD, *this))
+      return false;
+  }
+
+  if (VD->getAttrs().hasAttribute<ImplementationOnlyAttr>())
+    return false;
+
+  if (isAsyncAlternativeOfOtherDecl(VD))
+    return false;
+
+  if (excludeForObjCImplementation(VD))
+    return false;
+
+  return true;
 }
 
 bool DeclAndTypePrinter::isVisible(const ValueDecl *vd) const {
diff --git a/lib/PrintAsClang/PrintClangFunction.cpp b/lib/PrintAsClang/PrintClangFunction.cpp
@@ -102,18 +102,45 @@ struct CFunctionSignatureTypePrinterModifierDelegate {
 
 class ClangTypeHandler {
 public:
-  ClangTypeHandler(const clang::Decl *typeDecl) : typeDecl(typeDecl) {}
+  ClangTypeHandler(const clang::Decl *typeDecl)
+      : typeDecl(dyn_cast<clang::TagDecl>(typeDecl)) {}
 
   bool isRepresentable() const {
-    // We can only return trivial types, or
-    // types that can be moved or copied.
-    if (auto *record = dyn_cast<clang::CXXRecordDecl>(typeDecl)) {
-      return record->isTrivial() || record->hasMoveConstructor() ||
-             record->hasCopyConstructorWithConstParam();
+    // We can only return tag types.
+    if (typeDecl) {
+      // We can return trivial types.
+      if (isTrivial(typeDecl))
+        return true;
+
+      // We can return nontrivial types iff they can be moved or copied.
+      if (auto *record = dyn_cast<clang::CXXRecordDecl>(typeDecl)) {
+        return record->hasMoveConstructor() ||
+               record->hasCopyConstructorWithConstParam();
+      }
     }
+
+    // Otherwise, we can't return this type.
     return false;
   }
 
+private:
+  /// Is the tag type trivial?
+  static bool isTrivial(const clang::TagDecl *typeDecl) {
+    if (!typeDecl)
+      return false;
+
+    if (auto *record = dyn_cast<clang::CXXRecordDecl>(typeDecl))
+      return record->isTrivial();
+
+    // FIXME: If we can get plain clang::RecordDecls here, we need to figure out
+    //        how nontrivial (i.e. ARC) fields work.
+    assert(!isa<clang::RecordDecl>(typeDecl));
+
+    // C-family enums are always trivial.
+    return isa<clang::EnumDecl>(typeDecl);
+  }
+
+public:
   void printTypeName(raw_ostream &os) const {
     ClangSyntaxPrinter(os).printClangTypeReference(typeDecl);
   }
@@ -133,15 +160,15 @@ class ClangTypeHandler {
       llvm::raw_string_ostream typeNameOS(fullQualifiedType);
       printTypeName(typeNameOS);
       llvm::raw_string_ostream unqualTypeNameOS(typeName);
-      unqualTypeNameOS << cast<clang::NamedDecl>(typeDecl)->getName();
+      unqualTypeNameOS << typeDecl->getName();
     }
     printReturnScaffold(typeDecl, os, fullQualifiedType, typeName,
                         bodyOfReturn);
   }
 
 private:
   static void
-  printReturnScaffold(const clang::Decl *typeDecl, raw_ostream &os,
+  printReturnScaffold(const clang::TagDecl *typeDecl, raw_ostream &os,
                       StringRef fullQualifiedType, StringRef typeName,
                       llvm::function_ref<void(StringRef)> bodyOfReturn) {
     os << "alignas(alignof(" << fullQualifiedType << ")) char storage[sizeof("
@@ -150,7 +177,7 @@ class ClangTypeHandler {
        << fullQualifiedType << " *>(storage);\n";
     bodyOfReturn("storage");
     os << ";\n";
-    if (typeDecl && cast<clang::CXXRecordDecl>(typeDecl)->isTrivial()) {
+    if (isTrivial(typeDecl)) {
       // Trivial object can be just copied and not destroyed.
       os << "return *storageObjectPtr;\n";
       return;
@@ -162,7 +189,7 @@ class ClangTypeHandler {
     os << "return result;\n";
   }
 
-  const clang::Decl *typeDecl;
+  const clang::TagDecl *typeDecl;
 };
 
 // Prints types in the C function signature that corresponds to the
@@ -371,6 +398,7 @@ class CFunctionSignatureTypePrinter
       return ClangRepresentation::unsupported;
 
     if (decl->hasClangNode()) {
+      assert(genericArgs.empty() && "this path doesn't support generic args");
       ClangTypeHandler handler(decl->getClangDecl());
       if (!handler.isRepresentable())
         return ClangRepresentation::unsupported;
diff --git a/test/Interop/CxxToSwiftToCxx/bridge-cxx-struct-back-to-cxx.swift b/test/Interop/CxxToSwiftToCxx/bridge-cxx-struct-back-to-cxx.swift
@@ -3,11 +3,11 @@
 
 // RUN: %target-swift-frontend -typecheck %t/use-cxx-types.swift -typecheck -module-name UseCxxTy -emit-clang-header-path %t/UseCxxTy.h -I %t -enable-experimental-cxx-interop -clang-header-expose-decls=all-public -disable-availability-checking
 
-// RUN: %FileCheck %s < %t/UseCxxTy.h
+// RUN: %FileCheck %s --input-file %t/UseCxxTy.h
 
 // RUN: %target-swift-frontend -typecheck %t/use-cxx-types.swift -typecheck -module-name UseCxxTy -emit-clang-header-path %t/UseCxxTyExposeOnly.h -I %t -enable-experimental-cxx-interop -clang-header-expose-decls=has-expose-attr -disable-availability-checking
 
-// RUN: %FileCheck %s < %t/UseCxxTyExposeOnly.h
+// RUN: %FileCheck %s --input-file %t/UseCxxTyExposeOnly.h
 
 // FIXME: remove once https://github.com/apple/swift/pull/60971 lands.
 // RUN: echo "#include \"header.h\"" > %t/full-cxx-swift-cxx-bridging.h
@@ -91,6 +91,9 @@ using anonStructInNS = struct { float row; };
 
 }
 
+enum class SimpleScopedEnum { x = 0, y = 2 };
+typedef SimpleScopedEnum SimpleScopedEnumTypedef;
+
 //--- module.modulemap
 module CxxTest {
     header "header.h"
@@ -130,6 +133,16 @@ public func retNonTrivialTypeAlias() -> ns.TypeAlias {
     return ns.TypeAlias()
 }
 
+@_expose(Cxx)
+public func retSimpleScopedEnum() -> SimpleScopedEnum {
+  return .x
+}
+
+@_expose(Cxx)
+public func retSimpleScopedEnumTypedef() -> SimpleScopedEnumTypedef {
+  return .x
+}
+
 @_expose(Cxx)
 public func retSimpleTypedef() -> SimpleTypedef {
     return SimpleTypedef()
@@ -152,6 +165,10 @@ public func takeImmortalTemplate(_ x: ns.ImmortalCInt) {
 public func takeNonTrivial2(_ x: ns.NonTrivialTemplateTrivial) {
 }
 
+@_expose(Cxx)
+public func takeSimpleScopedEnum(_ x: SimpleScopedEnum) {
+}
+
 @_expose(Cxx)
 public func takeTrivial(_ x: Trivial) {
 }
@@ -275,6 +292,12 @@ public struct Strct {
 
 // CHECK: ns::NonTrivialTemplate<ns::TrivialinNS> retNonTrivialTypeAlias() noexcept SWIFT_SYMBOL({{.*}}) SWIFT_WARN_UNUSED_RESULT {
 
+// CHECK: SimpleScopedEnum retSimpleScopedEnum() noexcept SWIFT_SYMBOL({{.*}}) SWIFT_WARN_UNUSED_RESULT {
+
+// FIXME: Would we prefer to print these with the typedef names?
+// CHECK: SimpleScopedEnum retSimpleScopedEnumTypedef() noexcept SWIFT_SYMBOL({{.*}}) SWIFT_WARN_UNUSED_RESULT {
+// CHECK: int32_t retSimpleTypedef() noexcept SWIFT_SYMBOL({{.*}}) SWIFT_WARN_UNUSED_RESULT {
+
 // CHECK: SWIFT_INLINE_THUNK Trivial retTrivial() noexcept SWIFT_SYMBOL({{.*}}) SWIFT_WARN_UNUSED_RESULT {
 // CHECK-NEXT: alignas(alignof(Trivial)) char storage[sizeof(Trivial)];
 // CHECK-NEXT: auto * _Nonnull storageObjectPtr = reinterpret_cast<Trivial *>(storage);
@@ -294,6 +317,8 @@ public struct Strct {
 // CHECK-NEXT:   _impl::$s8UseCxxTy15takeNonTrivial2yySo2nsO0037NonTrivialTemplateTrivialinNS_CsGGkdcVF(swift::_impl::getOpaquePointer(x));
 // CHECK-NEXT: }
 
+// CHECK: SWIFT_INLINE_THUNK void takeSimpleScopedEnum(const SimpleScopedEnum& x) noexcept SWIFT_SYMBOL({{.*}}) {
+
 // CHECK: SWIFT_INLINE_THUNK void takeTrivial(const Trivial& x) noexcept SWIFT_SYMBOL({{.*}}) {
 // CHECK-NEXT:   _impl::$s8UseCxxTy11takeTrivialyySo0E0VF(_impl::swift_interop_passDirect_UseCxxTy_uint32_t_0_4(reinterpret_cast<const char *>(swift::_impl::getOpaquePointer(x))));
 // CHECK-NEXT: }
