Skip to content

[PrintAsCxx] Fix printing of C++ enum args #74637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions lib/AST/SwiftNameTranslation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand All @@ -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};
Expand Down
38 changes: 28 additions & 10 deletions lib/PrintAsClang/DeclAndTypePrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change to this function is NFC; I simply found the gigantic (and ever-growing, if you look at its version history!) expression hard to understand and even harder to debug.

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 {
Expand Down
48 changes: 38 additions & 10 deletions lib/PrintAsClang/PrintClangFunction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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("
Expand All @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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()
Expand All @@ -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) {
}
Expand Down Expand Up @@ -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);
Expand All @@ -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: }
Expand Down