Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ClangImporter: Allow for structs with arc pointers to be constructed in objc mode #64381

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

ClangImporter: Allow for structs with arc pointers to be constructed in objc mode #64381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1814fde
ClangImporter: Allow for structs with arc pointers to be constructed …
AdamCmiel Mar 13, 2023
402ffe7
[WIP] finalizing pointers
AdamCmiel Mar 22, 2023
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
1 change: 1 addition & 0 deletions include/swift/AST/DiagnosticsClangImporter.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,7 @@ ERROR(move_only_requires_move_only,none,
ERROR(failed_base_method_call_synthesis,none,
"failed to synthesize call to the base method %0 of type %0",
(ValueDecl *, ValueDecl *))
WARNING(record_field_weak_nonnull, none, "field %0 cannot import as both __weak and _Nonnull", (const clang::NamedDecl*))

NOTE(unsupported_builtin_type, none, "built-in type '%0' not supported", (StringRef))
NOTE(record_field_not_imported, none, "field %0 unavailable (cannot import)", (const clang::NamedDecl*))
Expand Down
115 changes: 83 additions & 32 deletions lib/ClangImporter/ImportDecl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -775,6 +775,13 @@ classImplementsProtocol(const clang::ObjCInterfaceDecl *constInterface,
return interface->ClassImplementsProtocol(proto, checkCategories);
}

static void
setWeakStorageInterface(VarDecl *var, Type type) {
ASTContext &ctx = var->getASTContext();
var->getAttrs().add(new (ctx) ReferenceOwnershipAttr(ReferenceOwnership::Weak));
var->setInterfaceType(WeakStorageType::get(type->isOptional() ? type : type->wrapInOptionalType(), ctx));
}

static void
applyPropertyOwnership(VarDecl *prop,
clang::ObjCPropertyAttribute::Kind attrs) {
Expand All @@ -790,10 +797,7 @@ applyPropertyOwnership(VarDecl *prop,
return;
}
if (attrs & clang::ObjCPropertyAttribute::kind_weak) {
prop->getAttrs().add(new (ctx)
ReferenceOwnershipAttr(ReferenceOwnership::Weak));
prop->setInterfaceType(WeakStorageType::get(
prop->getInterfaceType(), ctx));
setWeakStorageInterface(prop, prop->getInterfaceType());
return;
}
if ((attrs & clang::ObjCPropertyAttribute::kind_assign) ||
Expand Down Expand Up @@ -2122,25 +2126,6 @@ namespace {
isNonTrivialPtrAuth = Impl.SwiftContext.SILOpts
.EnableImportPtrauthFieldFunctionPointers &&
isNonTrivialDueToAddressDiversifiedPtrAuth(decl);
if (!isNonTrivialPtrAuth) {
// Note that there is a third predicate related to these,
// isNonTrivialToPrimitiveDefaultInitialize. That one's not important
// for us because Swift never "trivially default-initializes" a struct
// (i.e. uses whatever bits were lying around as an initial value).

// FIXME: It would be nice to instead import the declaration but mark
// it as unavailable, but then it might get used as a type for an
// imported function and the developer would be able to use it without
// referencing the name, which would sidestep our availability
// diagnostics.
Impl.addImportDiagnostic(
decl,
Diagnostic(
diag::record_non_trivial_copy_destroy,
Impl.SwiftContext.AllocateCopy(decl->getNameAsString())),
decl->getLocation());
return nullptr;
}
}

// Import the name.
Expand Down Expand Up @@ -2273,6 +2258,7 @@ namespace {
}

Decl *member = Impl.importDecl(nd, getActiveSwiftVersion());


if (!member) {
if (!isa<clang::TypeDecl>(nd) && !isa<clang::FunctionDecl>(nd) &&
Expand All @@ -2285,6 +2271,14 @@ namespace {
}
continue;
}

// if (auto VD = dyn_cast<VarDecl>(member)) {
// if (auto FD = dyn_cast<clang::FieldDecl>(nd)) {
// if (isStrongAndBridgedObjCObjectPointer(VD)) {
// synthesizer.makeComputedFieldAccessors(FD, isConstQualified, result, VD);
// }
// }
// }

if (nd->getDeclName().isIdentifier())
allMemberNames.insert(nd->getName());
Expand Down Expand Up @@ -3219,6 +3213,41 @@ namespace {
return false;
}

static ImportTypeKind
importTypeKindForRecordFieldObjCLifetime(clang::Qualifiers::ObjCLifetime objCLifetime) {
switch (objCLifetime) {
case clang::Qualifiers::OCL_None:
case clang::Qualifiers::OCL_ExplicitNone:
case clang::Qualifiers::OCL_Weak:
return ImportTypeKind::RecordFieldWithReferenceSemantics;
case clang::Qualifiers::OCL_Strong:
return ImportTypeKind::RecordField;
// Fields cannot have autoreleasing membership
case clang::Qualifiers::OCL_Autoreleasing:
llvm_unreachable("invalid ObjC lifetime");
}
}

static Bridgeability
bridgeabilityForObjCFieldDecl(clang::Qualifiers::ObjCLifetime objCLifetime) {
switch (objCLifetime) {
case clang::Qualifiers::OCL_None:
case clang::Qualifiers::OCL_ExplicitNone:
case clang::Qualifiers::OCL_Weak:
return Bridgeability::None;
case clang::Qualifiers::OCL_Strong:
return Bridgeability::Full;
// Fields cannot have autoreleasing membership
case clang::Qualifiers::OCL_Autoreleasing:
llvm_unreachable("invalid ObjC lifetime");
}
}

static bool
isStrongAndBridgedObjCObjectPointer(VarDecl *decl) {
return false;
}

Decl *VisitFunctionDecl(const clang::FunctionDecl *decl) {
// Import the name of the function.
ImportedName importedName;
Expand Down Expand Up @@ -3857,38 +3886,60 @@ namespace {
auto fieldType = decl->getType();
ImportedType importedType = findOptionSetType(fieldType, Impl);

auto objcLifetime = decl->getType().getObjCLifetime();
if (!importedType)
importedType =
Impl.importType(decl->getType(), ImportTypeKind::RecordField,
ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
isInSystemModule(dc), Bridgeability::None,
getImportTypeAttrs(decl));
Impl.importType(decl->getType(),
importTypeKindForRecordFieldObjCLifetime(objcLifetime),
ImportDiagnosticAdder(Impl, decl, decl->getLocation()),
isInSystemModule(dc), bridgeabilityForObjCFieldDecl(objcLifetime),
getImportTypeAttrs(decl));

if (!importedType) {
Impl.addImportDiagnostic(
decl, Diagnostic(diag::record_field_not_imported, decl),
decl->getSourceRange().getBegin());
return nullptr;
}

auto type = importedType.getType();

auto result =
Impl.createDeclWithClangNode<VarDecl>(decl, AccessLevel::Public,
/*IsStatic*/ false,
VarDecl::Introducer::Var,
Impl.importSourceLoc(decl->getLocation()),
name, dc);

result->setInterfaceType(importedType.getType());
Impl.recordImplicitUnwrapForDecl(result,
importedType.isImplicitlyUnwrapped());

if (decl->getType().isConstQualified()) {
// Note that in C++ there are ways to change the values of const
// members, so we don't use WriteImplKind::Immutable storage.
assert(result->supportsMutation());
result->overwriteSetterAccess(AccessLevel::Private);
}

result->setIsObjC(false);
result->setIsDynamic(false);
result->setInterfaceType(type);
Impl.recordImplicitUnwrapForDecl(result,
importedType.isImplicitlyUnwrapped());

if (objcLifetime == clang::Qualifiers::OCL_Weak) {
if (auto nullability =
decl->getType()->getNullability(Impl.getClangASTContext()))
{
// If the struct field is weak and nonnull, do not provide an interface for this member.
if (*nullability == clang::NullabilityKind::NonNull) {
Impl.addImportDiagnostic(
Copy link
Contributor

Choose a reason for hiding this comment

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

We seen to be missing a test case that exercise this diagnostic.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also is it possible for an objc member to be weak and non-null? I thought weak was implicit always nullable. e.g. if we try to declare a property (weak, nonnull) we get property attributes 'nonnull' and 'weak' are mutually exclusive. So this is more a question because, mostly curious on this =]

Copy link
Contributor

Choose a reason for hiding this comment

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

nvm, saw the case bellow, surprised that this is allowed.

decl, Diagnostic(diag::record_field_weak_nonnull, decl),
decl->getSourceRange().getBegin());
return nullptr;
}
}

// If struct field is a weak objc object pointer, create a weak storage type
// and add the appropriate wrapper to the param
setWeakStorageInterface(result, importedType.getType());
}

// Handle attributes.
if (decl->hasAttr<clang::IBOutletAttr>())
Expand Down
18 changes: 6 additions & 12 deletions lib/ClangImporter/ImportType.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,8 +1277,9 @@ static bool canBridgeTypes(ImportTypeKind importKind) {
case ImportTypeKind::Variable:
case ImportTypeKind::AuditedVariable:
case ImportTypeKind::Enum:
case ImportTypeKind::RecordField:
case ImportTypeKind::RecordFieldWithReferenceSemantics:
return false;
case ImportTypeKind::RecordField:
case ImportTypeKind::Result:
case ImportTypeKind::AuditedResult:
case ImportTypeKind::Parameter:
Expand Down Expand Up @@ -1307,6 +1308,7 @@ static bool isCFAudited(ImportTypeKind importKind) {
case ImportTypeKind::Enum:
case ImportTypeKind::RecordField:
return false;
case ImportTypeKind::RecordFieldWithReferenceSemantics:
case ImportTypeKind::AuditedVariable:
case ImportTypeKind::AuditedResult:
case ImportTypeKind::Parameter:
Expand Down Expand Up @@ -1475,8 +1477,7 @@ static ImportedType adjustTypeForConcreteImport(
// bridge, do so.
if (canBridgeTypes(importKind) &&
importKind != ImportTypeKind::PropertyWithReferenceSemantics &&
!(importKind == ImportTypeKind::RecordField &&
objCLifetime <= clang::Qualifiers::OCL_ExplicitNone) &&
importKind != ImportTypeKind::RecordFieldWithReferenceSemantics &&
!(importKind == ImportTypeKind::Typedef &&
bridging == Bridgeability::None)) {
// id and Any can be bridged without Foundation. There would be
Expand Down Expand Up @@ -1594,7 +1595,7 @@ static ImportedType adjustTypeForConcreteImport(

assert(importedType);

if (importKind == ImportTypeKind::RecordField &&
if ((importKind == ImportTypeKind::RecordField || importKind == ImportTypeKind::RecordFieldWithReferenceSemantics) &&
!importedType->isForeignReferenceType()) {
switch (objCLifetime) {
// Wrap retainable struct fields in Unmanaged.
Expand All @@ -1606,16 +1607,9 @@ static ImportedType adjustTypeForConcreteImport(
importedType = getUnmanagedType(impl, importedType);
}
break;
// FIXME: Eventually we might get C++-like support for strong pointers in
// structs, at which point we should really be checking the lifetime
// qualifiers.
case clang::Qualifiers::OCL_Strong:
if (!impl.SwiftContext.LangOpts.EnableCXXInterop) {
return {Type(), false};
}
break;
case clang::Qualifiers::OCL_Weak:
return {Type(), false};
break;
case clang::Qualifiers::OCL_Autoreleasing:
llvm_unreachable("invalid Objective-C lifetime");
}
Expand Down
3 changes: 3 additions & 0 deletions lib/ClangImporter/ImporterImpl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ enum class ImportTypeKind {
/// Import the declared type of a struct or union field.
RecordField,

/// TODO
RecordFieldWithReferenceSemantics,

/// Import the result type of a function.
///
/// This provides special treatment for 'void', among other things, and
Expand Down
72 changes: 70 additions & 2 deletions lib/ClangImporter/SwiftDeclSynthesizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,12 +586,16 @@ ConstructorDecl *SwiftDeclSynthesizer::createValueConstructor(

bool generateParamName = wantCtorParamNames;

auto objcLifetime = clang::Qualifiers::OCL_None;
if (var->hasClangNode()) {
// TODO create value constructor with indirect fields instead of the
// generated __Anonymous_field.
if (isa<clang::IndirectFieldDecl>(var->getClangDecl()))
continue;

if (auto clangVal = dyn_cast<clang::ValueDecl>(var->getClangDecl()))
objcLifetime = clangVal->getType().getObjCLifetime();

if (auto clangField = dyn_cast<clang::FieldDecl>(var->getClangDecl()))
if (clangField->isAnonymousStructOrUnion() ||
clangField->getDeclName().isEmpty())
Expand All @@ -602,14 +606,25 @@ ConstructorDecl *SwiftDeclSynthesizer::createValueConstructor(
auto param =
new (context) ParamDecl(SourceLoc(), SourceLoc(), argName, SourceLoc(),
var->getName(), structDecl);

if (objcLifetime == clang::Qualifiers::OCL_Weak) {
// The synthesized constructor param shouldn't use a weak storage type
if (auto RST = dyn_cast<ReferenceStorageType>(var->getInterfaceType().getPointer())) {
// Unwrap the referent type to pass directly to the constructor
param->setInterfaceType(RST->getReferentType());
} else {
param->setInterfaceType(var->getInterfaceType());
}
} else {
param->setInterfaceType(var->getInterfaceType());
}

param->setSpecifier(ParamSpecifier::Default);
param->setInterfaceType(var->getInterfaceType());
ImporterImpl.recordImplicitUnwrapForDecl(
param, var->isImplicitlyUnwrappedOptional());

// Don't allow the parameter to accept temporary pointer conversions.
param->setNonEphemeralIfPossible();

valueParameters.push_back(param);
}

Expand Down Expand Up @@ -1199,6 +1214,59 @@ SwiftDeclSynthesizer::makeIndirectFieldAccessors(
return {getterDecl, setterDecl};
}

static std::pair<BraceStmt *, bool>
synthesizeComputedGetterFromField(AbstractFunctionDecl *afd, void *context) {
auto accessor = cast<AccessorDecl>(afd);
auto method = static_cast<FuncDecl *>(context);

auto selfArg = createSelfArg(accessor);

auto *getterImplCallExpr = createAccessorImplCallExpr(method, selfArg);
auto returnStmt =
new (method->getASTContext()) ReturnStmt(SourceLoc(), getterImplCallExpr);
auto body = BraceStmt::create(method->getASTContext(), SourceLoc(),
{returnStmt}, SourceLoc());

return {body, /*isTypeChecked*/ true};
}

//VarDecl *
//SwiftDeclSynthesizer::makeComputedPropertyFromField(
// const clang::FieldDecl *field, NominalTypeDecl *typeDecl, VarDecl *importedFieldDecl) {
// auto &Context = ImporterImpl.SwiftContext;
// const bool isConstQualified = field->getType().isConstQualified();
// importedFieldDecl
//
// // if is const qualified, don't synthesize a setter
// AccessorDecl *setterDecl = nullptr;
//
// AccessorDecl *getterDecl = AccessorDecl::create(Context, field->getLoc(), field->getLoc(), AccessorKind::Get, importedFieldDecl, SourceLoc(), StaticSpellingKind::None, /* async */ false, SourceLoc(), /* throws */ false, SourceLoc(), ParameterList::createEmpty(Context));
//
// getterDecl->setAccess(AccessLevel::Public);
// getterDecl->setImplicit();
// getterDecl->setIsDynamic(false);
// getterDecl->setIsTransparent(true);
// getterDecl->setBodySynthesizer(synthesizeComputedGetterFromField, field);
//
//
//// getterDecl->setBodySynthesizer(synthesizeIndirectFieldGetterBody,
//// anonymousFieldDecl);
//// setterDecl->setBodySynthesizer(synthesizeIndirectFieldSetterBody,
//// anonymousFieldDecl);
//
// if (isConstQualified) {
// VD->setImplInfo(StorageImplInfo::getImmutableComputed());
// } else {
// VD->setImplInfo(isConstQualified ? StorageImplInfo::getImmutableCompute)
// }
//
// Impl.makeComputed(VD, getter, setter);
// VD->setIsSetterMutating(!isConstQualified);
//
// // return VD
// return VD;
//}

// MARK: Enum RawValue initializers

/// Synthesize the body of \c init?(rawValue:RawType) for an imported enum.
Expand Down
7 changes: 7 additions & 0 deletions lib/ClangImporter/SwiftDeclSynthesizer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,13 @@ class SwiftDeclSynthesizer {
ArrayRef<VarDecl *> members,
NominalTypeDecl *importedStructDecl,
VarDecl *importedFieldDecl);

// /// Build the computed field getter and setter for bridged types
// ///
// /// \returns a pair of getter and setter function decls.
// std::pair<AccessorDecl *, AccessorDecl *>
// makeComputedFieldAccessors(
// const clang::FieldDecl *field, bool isConstQualified, NominalTypeDecl *typeDecl, VarDecl *importedFieldDecl);

/// Build the init(rawValue:) initializer for an imported NS_ENUM.
///
Expand Down
Loading