Skip to content

[cxx-interop] Fix assertion failure from unavailable typedef + enum pattern #81625

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
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
55 changes: 6 additions & 49 deletions lib/ClangImporter/ImportDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -847,31 +847,6 @@ static bool isPrintLikeMethod(DeclName name, const DeclContext *dc) {
using MirroredMethodEntry =
std::tuple<const clang::ObjCMethodDecl*, ProtocolDecl*, bool /*isAsync*/>;

ImportedType findOptionSetType(clang::QualType type,
ClangImporter::Implementation &Impl) {
ImportedType importedType;
auto fieldType = type;
if (auto elaborated = dyn_cast<clang::ElaboratedType>(fieldType))
fieldType = elaborated->desugar();
if (auto typedefType = dyn_cast<clang::TypedefType>(fieldType)) {
if (Impl.isUnavailableInSwift(typedefType->getDecl())) {
if (auto clangEnum =
findAnonymousEnumForTypedef(Impl.SwiftContext, typedefType)) {
// If this fails, it means that we need a stronger predicate for
// determining the relationship between an enum and typedef.
assert(
clangEnum.value()->getIntegerType()->getCanonicalTypeInternal() ==
typedefType->getCanonicalTypeInternal());
if (auto swiftEnum = Impl.importDecl(*clangEnum, Impl.CurrentVersion)) {
importedType = {cast<TypeDecl>(swiftEnum)->getDeclaredInterfaceType(),
false};
}
}
}
}
return importedType;
}

static bool areRecordFieldsComplete(const clang::CXXRecordDecl *decl) {
for (const auto *f : decl->fields()) {
auto *fieldRecord = f->getType()->getAsCXXRecordDecl();
Expand Down Expand Up @@ -4511,8 +4486,8 @@ namespace {
return nullptr;
}

auto fieldType = decl->getType();
ImportedType importedType = findOptionSetType(fieldType, Impl);
auto fieldType = desugarIfElaborated(decl->getType());
ImportedType importedType = importer::findOptionSetEnum(fieldType, Impl);

if (!importedType)
importedType =
Expand Down Expand Up @@ -6155,8 +6130,8 @@ namespace {
}
}

auto fieldType = decl->getType();
ImportedType importedType = findOptionSetType(fieldType, Impl);
auto fieldType = desugarIfElaborated(decl->getType());
ImportedType importedType = importer::findOptionSetEnum(fieldType, Impl);

if (!importedType)
importedType = Impl.importPropertyType(decl, isInSystemModule(dc));
Expand Down Expand Up @@ -7120,8 +7095,7 @@ SwiftDeclConverter::getImplicitProperty(ImportedName importedName,
getAccessorPropertyType(getter, false, getterName.getSelfIndex());
if (propertyType.isNull())
return nullptr;
if (auto elaborated = dyn_cast<clang::ElaboratedType>(propertyType))
propertyType = elaborated->desugar();
propertyType = desugarIfElaborated(propertyType);

// If there is a setter, check that the property it implies
// matches that of the getter.
Expand All @@ -7147,24 +7121,7 @@ SwiftDeclConverter::getImplicitProperty(ImportedName importedName,
if (dc->isTypeContext() && !getterName.getSelfIndex())
isStatic = true;

ImportedType importedType;

// Sometimes we import unavailable typedefs as enums. If that's the case,
// use the enum, not the typedef here.
if (auto typedefType = dyn_cast<clang::TypedefType>(propertyType.getTypePtr())) {
if (Impl.isUnavailableInSwift(typedefType->getDecl())) {
if (auto clangEnum = findAnonymousEnumForTypedef(Impl.SwiftContext, typedefType)) {
// If this fails, it means that we need a stronger predicate for
// determining the relationship between an enum and typedef.
assert(clangEnum.value()->getIntegerType()->getCanonicalTypeInternal() ==
typedefType->getCanonicalTypeInternal());
if (auto swiftEnum = Impl.importDecl(*clangEnum, Impl.CurrentVersion)) {
importedType = {cast<TypeDecl>(swiftEnum)->getDeclaredInterfaceType(),
false};
}
}
}
}
ImportedType importedType = importer::findOptionSetEnum(propertyType, Impl);

if (!importedType) {
// Compute the property type.
Expand Down
37 changes: 33 additions & 4 deletions lib/ClangImporter/ImportEnumInfo.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -244,10 +244,39 @@ StringRef importer::getCommonPluralPrefix(StringRef singular,
}

const clang::Type *importer::getUnderlyingType(const clang::EnumDecl *decl) {
const clang::Type *underlyingType = decl->getIntegerType().getTypePtr();
if (auto elaborated = dyn_cast<clang::ElaboratedType>(underlyingType))
underlyingType = elaborated->desugar().getTypePtr();
return underlyingType;
return importer::desugarIfElaborated(decl->getIntegerType().getTypePtr());
}

ImportedType importer::findOptionSetEnum(clang::QualType type,
ClangImporter::Implementation &Impl) {
auto typedefType = dyn_cast<clang::TypedefType>(type);
if (!typedefType || !Impl.isUnavailableInSwift(typedefType->getDecl()))
// If this isn't a typedef, or it is a typedef that is available in Swift,
// then this definitely isn't used for {CF,NS}_OPTIONS.
return ImportedType();

auto clangEnum = findAnonymousEnumForTypedef(Impl.SwiftContext, typedefType);
if (!clangEnum)
return ImportedType();

// Assert that the typedef has the same underlying integer representation as
// the enum we think it assigns a type name to.
//
// If these fails, it means that we need a stronger predicate for
// determining the relationship between an enum and typedef.
if (auto *tdEnum =
dyn_cast<clang::EnumType>(typedefType->getCanonicalTypeInternal())) {
ASSERT(clangEnum.value()->getIntegerType()->getCanonicalTypeInternal() ==
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess having these as ASSERT over assert might cause these to start failing in release builds, which might be a bit risky for 6.2... I don't have a strong opinion on this.

tdEnum->getDecl()->getIntegerType()->getCanonicalTypeInternal());
} else {
ASSERT(clangEnum.value()->getIntegerType()->getCanonicalTypeInternal() ==
typedefType->getCanonicalTypeInternal());
}

if (auto *swiftEnum = Impl.importDecl(*clangEnum, Impl.CurrentVersion))
return {cast<TypeDecl>(swiftEnum)->getDeclaredInterfaceType(), false};

return ImportedType();
}

/// Determine the prefix to be stripped from the names of the enum constants
Expand Down
99 changes: 16 additions & 83 deletions lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2282,10 +2282,7 @@ ImportedType ClangImporter::Implementation::importFunctionReturnType(
OptionalityOfReturn = OTK_ImplicitlyUnwrappedOptional;
}

clang::QualType returnType = clangDecl->getReturnType();
if (auto elaborated =
dyn_cast<clang::ElaboratedType>(returnType))
returnType = elaborated->desugar();
clang::QualType returnType = desugarIfElaborated(clangDecl->getReturnType());
// In C interop mode, the return type of library builtin functions
// like 'memcpy' from headers like 'string.h' drops
// any nullability specifiers from their return type, and preserves it on the
Expand Down Expand Up @@ -2323,19 +2320,9 @@ ImportedType ClangImporter::Implementation::importFunctionReturnType(
->isTemplateTypeParmType())
OptionalityOfReturn = OTK_None;

if (auto typedefType = dyn_cast<clang::TypedefType>(returnType)) {
if (isUnavailableInSwift(typedefType->getDecl())) {
if (auto clangEnum = findAnonymousEnumForTypedef(SwiftContext, typedefType)) {
// If this fails, it means that we need a stronger predicate for
// determining the relationship between an enum and typedef.
assert(clangEnum.value()->getIntegerType()->getCanonicalTypeInternal() ==
typedefType->getCanonicalTypeInternal());
if (auto swiftEnum = importDecl(*clangEnum, CurrentVersion)) {
return {cast<TypeDecl>(swiftEnum)->getDeclaredInterfaceType(), false};
}
}
}
}
ImportedType optionSetEnum = importer::findOptionSetEnum(returnType, *this);
if (optionSetEnum)
return optionSetEnum;

// Import the underlying result type.
if (clangDecl) {
Expand Down Expand Up @@ -2399,27 +2386,11 @@ ImportedType ClangImporter::Implementation::importFunctionParamsAndReturnType(

// Only eagerly import the return type if it's not too expensive (the current
// heuristic for that is if it's not a record type).
ImportedType importedType;
ImportDiagnosticAdder addDiag(*this, clangDecl,
clangDecl->getSourceRange().getBegin());
clang::QualType returnType = clangDecl->getReturnType();
if (auto elaborated = dyn_cast<clang::ElaboratedType>(returnType))
returnType = elaborated->desugar();

if (auto typedefType = dyn_cast<clang::TypedefType>(returnType)) {
if (isUnavailableInSwift(typedefType->getDecl())) {
if (auto clangEnum = findAnonymousEnumForTypedef(SwiftContext, typedefType)) {
// If this fails, it means that we need a stronger predicate for
// determining the relationship between an enum and typedef.
assert(clangEnum.value()->getIntegerType()->getCanonicalTypeInternal() ==
typedefType->getCanonicalTypeInternal());
if (auto swiftEnum = importDecl(*clangEnum, CurrentVersion)) {
importedType = {cast<TypeDecl>(swiftEnum)->getDeclaredInterfaceType(),
false};
}
}
}
}
clang::QualType returnType = desugarIfElaborated(clangDecl->getReturnType());

ImportedType importedType = importer::findOptionSetEnum(returnType, *this);

if (auto templateType =
dyn_cast<clang::TemplateTypeParmType>(returnType)) {
Expand Down Expand Up @@ -2483,9 +2454,7 @@ ClangImporter::Implementation::importParameterType(
std::optional<unsigned> completionHandlerErrorParamIndex,
ArrayRef<GenericTypeParamDecl *> genericParams,
llvm::function_ref<void(Diagnostic &&)> addImportDiagnosticFn) {
auto paramTy = param->getType();
if (auto elaborated = dyn_cast<clang::ElaboratedType>(paramTy))
paramTy = elaborated->desugar();
auto paramTy = desugarIfElaborated(param->getType());

ImportTypeKind importKind = paramIsCompletionHandler
? ImportTypeKind::CompletionHandlerParameter
Expand All @@ -2498,23 +2467,8 @@ ClangImporter::Implementation::importParameterType(
bool isConsuming = false;
bool isParamTypeImplicitlyUnwrapped = false;

// Sometimes we import unavailable typedefs as enums. If that's the case,
// use the enum, not the typedef here.
if (auto typedefType = dyn_cast<clang::TypedefType>(paramTy.getTypePtr())) {
if (isUnavailableInSwift(typedefType->getDecl())) {
if (auto clangEnum =
findAnonymousEnumForTypedef(SwiftContext, typedefType)) {
// If this fails, it means that we need a stronger predicate for
// determining the relationship between an enum and typedef.
assert(clangEnum.value()
->getIntegerType()
->getCanonicalTypeInternal() ==
typedefType->getCanonicalTypeInternal());
if (auto swiftEnum = importDecl(*clangEnum, CurrentVersion)) {
swiftParamTy = cast<TypeDecl>(swiftEnum)->getDeclaredInterfaceType();
}
}
}
if (auto optionSetEnum = importer::findOptionSetEnum(paramTy, *this)) {
swiftParamTy = optionSetEnum.getType();
} else if (isa<clang::PointerType>(paramTy) &&
isa<clang::TemplateTypeParmType>(paramTy->getPointeeType())) {
auto pointeeType = paramTy->getPointeeType();
Expand Down Expand Up @@ -3234,26 +3188,8 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(

ImportDiagnosticAdder addImportDiag(*this, clangDecl,
clangDecl->getLocation());
clang::QualType resultType = clangDecl->getReturnType();
if (auto elaborated = dyn_cast<clang::ElaboratedType>(resultType))
resultType = elaborated->desugar();

ImportedType importedType;
if (auto typedefType = dyn_cast<clang::TypedefType>(resultType.getTypePtr())) {
if (isUnavailableInSwift(typedefType->getDecl())) {
if (auto clangEnum = findAnonymousEnumForTypedef(SwiftContext, typedefType)) {
// If this fails, it means that we need a stronger predicate for
// determining the relationship between an enum and typedef.
assert(clangEnum.value()->getIntegerType()->getCanonicalTypeInternal() ==
typedefType->getCanonicalTypeInternal());
if (auto swiftEnum = importDecl(*clangEnum, CurrentVersion)) {
importedType = {cast<TypeDecl>(swiftEnum)->getDeclaredInterfaceType(),
false};
}
}
}
}

clang::QualType resultType = desugarIfElaborated(clangDecl->getReturnType());
ImportedType importedType = importer::findOptionSetEnum(resultType, *this);
if (!importedType)
importedType = importType(resultType, resultKind, addImportDiag,
allowNSUIntegerAsIntInResult, Bridgeability::Full,
Expand Down Expand Up @@ -3501,9 +3437,6 @@ ImportedType ClangImporter::Implementation::importMethodParamsAndReturnType(
importedType.isImplicitlyUnwrapped()};
}

ImportedType findOptionSetType(clang::QualType type,
ClangImporter::Implementation &Impl);

ImportedType ClangImporter::Implementation::importAccessorParamsAndReturnType(
const DeclContext *dc, const clang::ObjCPropertyDecl *property,
const clang::ObjCMethodDecl *clangDecl, bool isFromSystemModule,
Expand All @@ -3529,11 +3462,11 @@ ImportedType ClangImporter::Implementation::importAccessorParamsAndReturnType(
if (!origDC)
return {Type(), false};

auto fieldType = isGetter ? clangDecl->getReturnType()
: clangDecl->getParamDecl(0)->getType();

auto fieldType =
desugarIfElaborated(isGetter ? clangDecl->getReturnType()
: clangDecl->getParamDecl(0)->getType());
// Import the property type, independent of what kind of accessor this is.
ImportedType importedType = findOptionSetType(fieldType, *this);
ImportedType importedType = importer::findOptionSetEnum(fieldType, *this);
if (!importedType)
importedType = importPropertyType(property, isFromSystemModule);
if (!importedType)
Expand Down
21 changes: 21 additions & 0 deletions lib/ClangImporter/ImporterImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -2085,6 +2085,27 @@ bool hasEscapableAttr(const clang::RecordDecl *decl);

bool isViewType(const clang::CXXRecordDecl *decl);

inline const clang::Type *desugarIfElaborated(const clang::Type *type) {
if (auto elaborated = dyn_cast<clang::ElaboratedType>(type))
return elaborated->desugar().getTypePtr();
return type;
}

inline clang::QualType desugarIfElaborated(clang::QualType type) {
if (auto elaborated = dyn_cast<clang::ElaboratedType>(type))
return elaborated->desugar();
return type;
}

/// Option set enums are sometimes imported as typedefs which assign a name to
/// the type, but are unavailable in Swift.
///
/// If given such a typedef, this helper function retrieves and imports the
/// underlying enum type. Returns an empty ImportedType otherwise.
///
/// If \a type is an elaborated type, it should be desugared first.
ImportedType findOptionSetEnum(clang::QualType type,
ClangImporter::Implementation &Impl);
} // end namespace importer
} // end namespace swift

Expand Down
60 changes: 60 additions & 0 deletions test/ClangImporter/nsenum-with-unavailable-typedef.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
// RUN: split-file %s %t
// RUN: %target-swift-frontend -typecheck -verify %t/main.swift -I %t/include -module-name main
// RUN: %target-swift-frontend -typecheck -verify %t/main.swift -I %t/include -module-name main -cxx-interoperability-mode=default
// REQUIRES: objc_interop

//--- include/module.modulemap
module ObjCModule {
header "header.h"
}

//--- include/header.h
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we have this test somewhere around test/Interop/Cxx/objc-correctness/Inputs/CFOptions.h?

#include <Foundation/Foundation.h>

#define UNAVAILABLE API_UNAVAILABLE(macos)

UNAVAILABLE
typedef NS_ENUM(NSUInteger, MyUnavailableEnum) {
MyUnavailableEnumCase1 = 0,
MyUnavailableEnumCase2,
};

UNAVAILABLE
void unavailableParam(MyUnavailableEnum e);

UNAVAILABLE
MyUnavailableEnum unavailableReturn(void);

struct UNAVAILABLE UnavailableStruct {
MyUnavailableEnum field;
};

UNAVAILABLE
@interface UnavailableClass
@property (nonatomic, readonly) MyUnavailableEnum prop;
@end

UNAVAILABLE
__attribute__((swift_name("getter:UnavailableStruct.iProp(self:)")))
inline MyUnavailableEnum getUnavailableInjProp(struct UnavailableStruct s) {
return MyUnavailableEnumCase1;
}

//--- main.swift
import Foundation
import ObjCModule

@available(*, unavailable)
func useParam(_ e: MyUnavailableEnum) { unavailableParam(e) }

@available(*, unavailable)
func useReturn() -> MyUnavailableEnum { return unavailableReturn() }

@available(*, unavailable)
func useField(_ o: UnavailableStruct) -> MyUnavailableEnum { return o.field }

@available(*, unavailable)
func useIProp(_ o: UnavailableStruct) -> MyUnavailableEnum { return o.iProp }

@available(*, unavailable)
func useProp(_ o: UnavailableClass) -> MyUnavailableEnum { return o.prop }