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.

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

SE-0055: Making pointer nullability explicit #1878

Merged
merged 5 commits into from
Apr 12, 2016
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
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,29 @@ Note: This is in reverse chronological order, so newer entries are added to the
Swift 3.0
-------

* As part of the changes for SE-0055 (see below), the *pointee* types of
imported pointers (e.g. the `id` in `id *`) are no longer assumed to always
be `_Nullable` even if annotated otherwise. However, an implicit or explicit
annotation of `_Null_unspecified` on a pointee type is still imported as
`Optional`.

* [SE-0055](https://github.com/apple/swift-evolution/blob/master/proposals/0055-optional-unsafe-pointers.md):
The types `UnsafePointer`, `UnsafeMutablePointer`,
`AutoreleasingUnsafeMutablePointer`, `OpaquePointer`, `Selector`, and `Zone`
(formerly `NSZone`) now represent non-nullable pointers, i.e. pointers that
are never `nil`. A nullable pointer is now represented using `Optional`, e.g.
`UnsafePointer<Int>?` For types imported from C, non-object pointers (such as
`int *`) now have their nullability taken into account.

One possible area of difficulty is passing a nullable pointer to a function
that uses C variadics. Swift will not permit this directly, so as a
workaround please use the following idiom to pass it as a pointer-sized
integer value instead:

```swift
unsafeBitCast(nullablePointer, to: Int.self)
```

* [SE-0046] (https://github.com/apple/swift-evolution/blob/master/proposals/0046-first-label.md) Function parameters now have consistent labelling across all function parameters. With this update the first parameter declarations will now match the existing behavior of the second and later parameters. This change makes the language simpler.

Functions that were written and called as follows
Expand Down
5 changes: 5 additions & 0 deletions include/swift/Runtime/Metadata.h
Original file line number Diff line number Diff line change
Expand Up @@ -907,6 +907,9 @@ extern "C" const ValueWitnessTable _TWVXwGSqBo_; // weak Builtin.NativeObject?
SWIFT_RUNTIME_EXPORT
extern "C" const ExtraInhabitantsValueWitnessTable _TWVBb; // Builtin.BridgeObject

SWIFT_RUNTIME_EXPORT
extern "C" const ExtraInhabitantsValueWitnessTable _TWVBp; // Builtin.RawPointer

#if SWIFT_OBJC_INTEROP
// The ObjC-pointer table can be used for arbitrary ObjC pointer types.
SWIFT_RUNTIME_EXPORT
Expand Down Expand Up @@ -1288,6 +1291,8 @@ extern "C" const FullOpaqueMetadata _TMBo; // Builtin.NativeObject
SWIFT_RUNTIME_EXPORT
extern "C" const FullOpaqueMetadata _TMBb; // Builtin.BridgeObject
SWIFT_RUNTIME_EXPORT
extern "C" const FullOpaqueMetadata _TMBp; // Builtin.RawPointer
SWIFT_RUNTIME_EXPORT
extern "C" const FullOpaqueMetadata _TMBB; // Builtin.UnsafeValueBuffer
#if SWIFT_OBJC_INTEROP
SWIFT_RUNTIME_EXPORT
Expand Down
6 changes: 3 additions & 3 deletions lib/AST/ASTContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3620,7 +3620,7 @@ ASTContext::getForeignRepresentationInfo(NominalTypeDecl *nominal,

// Pre-populate the foreign-representable cache with known types.
if (auto stdlib = getStdlibModule()) {
addTrivial(getIdentifier("OpaquePointer"), stdlib);
addTrivial(getIdentifier("OpaquePointer"), stdlib, true);

// Builtin types
// FIXME: Layering violation to use the ClangImporter's define.
Expand All @@ -3636,13 +3636,13 @@ ASTContext::getForeignRepresentationInfo(NominalTypeDecl *nominal,
}

if (auto objectiveC = getLoadedModule(Id_ObjectiveC)) {
addTrivial(Id_Selector, objectiveC);
addTrivial(Id_Selector, objectiveC, true);

// Note: ObjCBool is odd because it's bridged to Bool in APIs,
// but can also be trivially bridged.
addTrivial(getIdentifier("ObjCBool"), objectiveC);

addTrivial(getSwiftId(KnownFoundationEntity::NSZone), objectiveC);
addTrivial(getSwiftId(KnownFoundationEntity::NSZone), objectiveC, true);
}

if (auto coreGraphics = getLoadedModule(getIdentifier("CoreGraphics"))) {
Expand Down
4 changes: 0 additions & 4 deletions lib/AST/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2116,10 +2116,6 @@ getForeignRepresentable(Type type, ForeignLanguage language, DeclContext *dc) {
// Pointers may be representable in ObjC.
PointerTypeKind pointerKind;
if (auto pointerElt = type->getAnyPointerElementType(pointerKind)) {
// FIXME: Optionality should be embedded in the pointer types.
if (wasOptional)
return failure();

switch (pointerKind) {
case PTK_UnsafeMutablePointer:
case PTK_UnsafePointer:
Expand Down
56 changes: 26 additions & 30 deletions lib/ClangImporter/ImportType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,8 @@ namespace {
/// The source type is a function pointer type.
CFunctionPointer,

/// The source type is a specially-handled pointer type (usually a mapped
/// typedef) that nonetheless needs to preserve nullability.
CustomNullablePointer,
/// The source type is any other pointer type.
OtherPointer,
};

ImportHintKind Kind;
Expand Down Expand Up @@ -126,7 +125,7 @@ namespace {
case ImportHint::ObjCBridged:
case ImportHint::ObjCPointer:
case ImportHint::CFunctionPointer:
case ImportHint::CustomNullablePointer:
case ImportHint::OtherPointer:
return true;
}
}
Expand All @@ -151,17 +150,16 @@ namespace {
getOptionalType(Type payloadType,
ImportTypeKind kind,
OptionalTypeKind OptKind = OTK_ImplicitlyUnwrappedOptional) {
// Import pointee types as true Optional.
if (kind == ImportTypeKind::Pointee)
return OptionalType::get(payloadType);

switch (OptKind) {
case OTK_ImplicitlyUnwrappedOptional:
return ImplicitlyUnwrappedOptionalType::get(payloadType);
case OTK_None:
return payloadType;
case OTK_Optional:
case OTK_None:
return payloadType;
case OTK_Optional:
return OptionalType::get(payloadType);
case OTK_ImplicitlyUnwrappedOptional:
// Import pointee types as true Optional.
if (kind == ImportTypeKind::Pointee)
return OptionalType::get(payloadType);
return ImplicitlyUnwrappedOptionalType::get(payloadType);
}
}

Expand Down Expand Up @@ -292,7 +290,7 @@ namespace {
Impl.SwiftContext.getSwiftName(
KnownFoundationEntity::NSZone));
if (wrapperTy)
return wrapperTy;
return {wrapperTy, ImportHint::OtherPointer};
}
}

Expand All @@ -315,7 +313,8 @@ namespace {
// If the pointed-to type is unrepresentable in Swift, import as
// OpaquePointer.
if (!pointeeType)
return getOpaquePointerType();
return {Impl.SwiftContext.getOpaquePointerDecl()->getDeclaredType(),
ImportHint::OtherPointer};

if (pointeeQualType->isFunctionType()) {
auto funcTy = pointeeType->castTo<FunctionType>();
Expand All @@ -329,29 +328,29 @@ namespace {

auto quals = pointeeQualType.getQualifiers();

if (quals.hasConst())
if (quals.hasConst()) {
return {Impl.getNamedSwiftTypeSpecialization(Impl.getStdlibModule(),
"UnsafePointer",
pointeeType),
ImportHint::None};
ImportHint::OtherPointer};
}

// Mutable pointers with __autoreleasing or __unsafe_unretained
// ownership map to AutoreleasingUnsafeMutablePointer<T>.
else if (quals.getObjCLifetime() == clang::Qualifiers::OCL_Autoreleasing
|| quals.getObjCLifetime() == clang::Qualifiers::OCL_ExplicitNone)
if (quals.getObjCLifetime() == clang::Qualifiers::OCL_Autoreleasing ||
quals.getObjCLifetime() == clang::Qualifiers::OCL_ExplicitNone) {
return {
Impl.getNamedSwiftTypeSpecialization(
Impl.getStdlibModule(), "AutoreleasingUnsafeMutablePointer",
pointeeType),
ImportHint::None};
ImportHint::OtherPointer};
}

// All other mutable pointers map to UnsafeMutablePointer.
return {Impl.getNamedSwiftTypeSpecialization(Impl.getStdlibModule(),
"UnsafeMutablePointer",
pointeeType),
ImportHint::None};
}

Type getOpaquePointerType() {
return Impl.getNamedSwiftType(Impl.getStdlibModule(), "OpaquePointer");
ImportHint::OtherPointer};
}

ImportResult VisitBlockPointerType(const clang::BlockPointerType *type) {
Expand Down Expand Up @@ -568,11 +567,8 @@ namespace {
hint = ImportHint::CFPointer;
} else if (mappedType->isAnyExistentialType()) { // id, Class
hint = ImportHint::ObjCPointer;
} else if (type->isBlockPointerType()) {
// FIXME: This should eventually be "isAnyPointerType", but right now
// non-object, non-block pointers are never Optional in Swift; they
// just can have a value of 'nil' themselves.
hint = ImportHint::CustomNullablePointer;
} else if (type->isPointerType() || type->isBlockPointerType()) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note for reviewers: the comment was wrong. Clang's isAnyPointerType includes pointers-to-ObjC-objects (covered above) and does not include block pointers.

hint = ImportHint::OtherPointer;
}
// Any other interesting mapped types should be hinted here.

Expand Down
1 change: 1 addition & 0 deletions lib/IDE/CodeCompletion.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3309,6 +3309,7 @@ class CompletionLookup final : public swift::VisibleDeclConsumer {
// If the expected type is ObjectiveC.Selector, add #selector.
if (Ctx.LangOpts.EnableObjCInterop) {
for (auto T : ExpectedTypes) {
T = T->lookThroughAllAnyOptionalTypes();
if (auto structDecl = T->getStructOrBoundGenericStruct()) {
if (structDecl->getName() == Ctx.Id_Selector &&
structDecl->getParentModule()->getName() == Ctx.Id_ObjectiveC) {
Expand Down
3 changes: 2 additions & 1 deletion lib/IRGen/GenMeta.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1765,7 +1765,8 @@ namespace {
if (t == C.TheEmptyTupleType
|| t == C.TheNativeObjectType
|| t == C.TheUnknownObjectType
|| t == C.TheBridgeObjectType)
|| t == C.TheBridgeObjectType
|| t == C.TheRawPointerType)
return true;
if (auto intTy = dyn_cast<BuiltinIntegerType>(t)) {
auto width = intTy->getWidth();
Expand Down
63 changes: 62 additions & 1 deletion lib/IRGen/GenType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,53 @@ namespace {
Alignment align)
: PODSingleScalarTypeInfo(storage, size, std::move(spareBits), align) {}
};


/// A TypeInfo implementation for bare non-null pointers (like `void *`).
class RawPointerTypeInfo final :
public PODSingleScalarTypeInfo<RawPointerTypeInfo, LoadableTypeInfo> {
public:
RawPointerTypeInfo(llvm::Type *storage, Size size, Alignment align)
: PODSingleScalarTypeInfo(
storage, size,
SpareBitVector::getConstant(size.getValueInBits(), false),
align) {}

bool mayHaveExtraInhabitants(IRGenModule &IGM) const override {
return true;
}

unsigned getFixedExtraInhabitantCount(IRGenModule &IGM) const override {
return 1;
}

APInt getFixedExtraInhabitantValue(IRGenModule &IGM, unsigned bits,
unsigned index) const override {
assert(index == 0);
return APInt(bits, 0);
}

llvm::Value *getExtraInhabitantIndex(IRGenFunction &IGF,
Address src,
SILType T) const override {
// Copied from BridgeObjectTypeInfo.
src = IGF.Builder.CreateBitCast(src, IGF.IGM.IntPtrTy->getPointerTo());
auto val = IGF.Builder.CreateLoad(src);
auto zero = llvm::ConstantInt::get(IGF.IGM.IntPtrTy, 0);
auto isNonzero = IGF.Builder.CreateICmpNE(val, zero);
// We either have extra inhabitant 0 or no extra inhabitant (-1).
// Conveniently, this is just a sext i1 -> i32 away.
return IGF.Builder.CreateSExt(isNonzero, IGF.IGM.Int32Ty);
}

void storeExtraInhabitant(IRGenFunction &IGF, llvm::Value *index,
Address dest, SILType T) const override {
// Copied from BridgeObjectTypeInfo.
// There's only one extra inhabitant, 0.
dest = IGF.Builder.CreateBitCast(dest, IGF.IGM.IntPtrTy->getPointerTo());
IGF.Builder.CreateStore(llvm::ConstantInt::get(IGF.IGM.IntPtrTy, 0),dest);
}
};

/// A TypeInfo implementation for opaque storage. Swift will preserve any
/// data stored into this arbitrarily sized and aligned field, but doesn't
/// know anything about the data.
Expand Down Expand Up @@ -720,6 +766,20 @@ const LoadableTypeInfo &TypeConverter::getBridgeObjectTypeInfo() {
return *BridgeObjectTI;
}

const LoadableTypeInfo &IRGenModule::getRawPointerTypeInfo() {
return Types.getRawPointerTypeInfo();
}

const LoadableTypeInfo &TypeConverter::getRawPointerTypeInfo() {
if (RawPointerTI) return *RawPointerTI;
RawPointerTI = new RawPointerTypeInfo(IGM.Int8PtrTy,
IGM.getPointerSize(),
IGM.getPointerAlignment());
RawPointerTI->NextConverted = FirstType;
FirstType = RawPointerTI;
return *RawPointerTI;
}

const LoadableTypeInfo &TypeConverter::getEmptyTypeInfo() {
if (EmptyTI) return *EmptyTI;
EmptyTI = new EmptyTypeInfo(IGM.Int8Ty);
Expand Down Expand Up @@ -1263,6 +1323,7 @@ TypeCacheEntry TypeConverter::convertType(CanType ty) {
getFixedBufferSize(IGM),
getFixedBufferAlignment(IGM));
case TypeKind::BuiltinRawPointer:
return &getRawPointerTypeInfo();
case TypeKind::BuiltinFloat:
case TypeKind::BuiltinInteger:
case TypeKind::BuiltinVector: {
Expand Down
2 changes: 2 additions & 0 deletions lib/IRGen/GenType.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class TypeConverter {
const LoadableTypeInfo *NativeObjectTI = nullptr;
const LoadableTypeInfo *UnknownObjectTI = nullptr;
const LoadableTypeInfo *BridgeObjectTI = nullptr;
const LoadableTypeInfo *RawPointerTI = nullptr;
const LoadableTypeInfo *WitnessTablePtrTI = nullptr;
const LoadableTypeInfo *TypeMetadataPtrTI = nullptr;
const LoadableTypeInfo *ObjCClassPtrTI = nullptr;
Expand Down Expand Up @@ -148,6 +149,7 @@ class TypeConverter {
const LoadableTypeInfo &getNativeObjectTypeInfo();
const LoadableTypeInfo &getUnknownObjectTypeInfo();
const LoadableTypeInfo &getBridgeObjectTypeInfo();
const LoadableTypeInfo &getRawPointerTypeInfo();
const LoadableTypeInfo &getTypeMetadataPtrTypeInfo();
const LoadableTypeInfo &getObjCClassPtrTypeInfo();
const LoadableTypeInfo &getWitnessTablePtrTypeInfo();
Expand Down
1 change: 1 addition & 0 deletions lib/IRGen/IRGenModule.h
Original file line number Diff line number Diff line change
Expand Up @@ -550,6 +550,7 @@ class IRGenModule {
const LoadableTypeInfo &getNativeObjectTypeInfo();
const LoadableTypeInfo &getUnknownObjectTypeInfo();
const LoadableTypeInfo &getBridgeObjectTypeInfo();
const LoadableTypeInfo &getRawPointerTypeInfo();
llvm::Type *getStorageTypeForUnlowered(Type T);
llvm::Type *getStorageTypeForLowered(CanType T);
llvm::Type *getStorageType(SILType T);
Expand Down
24 changes: 5 additions & 19 deletions lib/PrintAsObjC/PrintAsObjC.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
// parameter, print it.
if (errorConvention && i == errorConvention->getErrorParameterIndex()) {
os << piece << ":(";
print(errorConvention->getErrorParameterType(), OTK_None);
print(errorConvention->getErrorParameterType(), None);
os << ")error";
continue;
}
Expand Down Expand Up @@ -918,10 +918,8 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
return false;

os << iter->second.first;
if (iter->second.second) {
// FIXME: Selectors and pointers should be nullable.
printNullability(OptionalTypeKind::OTK_ImplicitlyUnwrappedOptional);
}
if (iter->second.second)
printNullability(optionalKind);
return true;
}

Expand All @@ -932,13 +930,6 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
os << " */";
}

bool isClangObjectPointerType(const clang::TypeDecl *clangTypeDecl) const {
ASTContext &ctx = M.getASTContext();
auto &clangASTContext = ctx.getClangModuleLoader()->getClangASTContext();
clang::QualType clangTy = clangASTContext.getTypeDeclType(clangTypeDecl);
return clangTy->isObjCRetainableType();
}

bool isClangPointerType(const clang::TypeDecl *clangTypeDecl) const {
ASTContext &ctx = M.getASTContext();
auto &clangASTContext = ctx.getClangModuleLoader()->getClangASTContext();
Expand All @@ -958,13 +949,9 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
auto *clangTypeDecl = cast<clang::TypeDecl>(alias->getClangDecl());
os << clangTypeDecl->getName();

// Print proper nullability for CF types, but _Null_unspecified for
// all other non-object Clang pointer types.
if (aliasTy->hasReferenceSemantics() ||
isClangObjectPointerType(clangTypeDecl)) {
isClangPointerType(clangTypeDecl)) {
printNullability(optionalKind);
} else if (isClangPointerType(clangTypeDecl)) {
printNullability(OptionalTypeKind::OTK_ImplicitlyUnwrappedOptional);
}
return;
}
Expand Down Expand Up @@ -1069,8 +1056,7 @@ class ObjCPrinter : private DeclVisitor<ObjCPrinter>,
if (isConst)
os << " const";
os << " *";
// FIXME: Pointer types should be nullable.
printNullability(OptionalTypeKind::OTK_ImplicitlyUnwrappedOptional);
printNullability(optionalKind);
return true;
}

Expand Down
Loading