Skip to content

Commit

Permalink
Make pointer nullability explicit using Optional.
Browse files Browse the repository at this point in the history
Implements SE-0055: https://github.com/apple/swift-evolution/blob/master/proposals/0055-optional-unsafe-pointers.md

- Add NULL as an extra inhabitant of Builtin.RawPointer (currently
  hardcoded to 0 rather than being target-dependent).
- Import non-object pointers as Optional/IUO when nullable/null_unspecified
  (like everything else).
- Change the type checker's *-to-pointer conversions to handle a layer of
  optional.
- Use 'AutoreleasingUnsafeMutablePointer<NSError?>?' as the type of error
  parameters exported to Objective-C.
- Drop NilLiteralConvertible conformance for all pointer types.
- Update the standard library and then all the tests.

I've decided to leave this commit only updating existing tests; any new
tests will come in the following commits. (That may mean some additional
implementation work to follow.)

The other major piece that's missing here is migration. I'm hoping we get
a lot of that with Swift 1.1's work for optional object references, but
I still need to investigate.
  • Loading branch information
jrose-apple committed Apr 12, 2016
1 parent 8e292da commit bc83940
Show file tree
Hide file tree
Showing 150 changed files with 2,365 additions and 2,160 deletions.
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
39 changes: 18 additions & 21 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 Down Expand Up @@ -292,7 +291,7 @@ namespace {
Impl.SwiftContext.getSwiftName(
KnownFoundationEntity::NSZone));
if (wrapperTy)
return wrapperTy;
return {wrapperTy, ImportHint::OtherPointer};
}
}

Expand All @@ -315,7 +314,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 +329,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 +568,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()) {
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
8 changes: 4 additions & 4 deletions lib/SIL/SILFunctionType.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,10 @@ enum class ConventionsKind : uint8_t {
// (An ObjC class type wouldn't be const-qualified.)
if (clangTy->isPointerType()
&& clangTy->getPointeeType().isConstQualified()) {
// Peek through optionals.
if (auto substObjTy = substTy.getAnyOptionalObjectType())
substTy = substObjTy;

// Void pointers aren't usefully indirectable.
if (clangTy->isVoidPointerType())
return false;
Expand All @@ -395,10 +399,6 @@ enum class ConventionsKind : uint8_t {
// imported as methods.
return false;

// Peek through optionals.
if (auto substObjTy = substTy.getAnyOptionalObjectType())
substTy = substObjTy;

if (clangTy->getPointeeType()->getAs<clang::RecordType>()) {
// CF type as foreign class
if (substTy->getClassOrBoundGenericClass()
Expand Down
3 changes: 2 additions & 1 deletion lib/SILGen/SILGen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,10 @@ SILFunction *SILGenModule::emitTopLevelFunction(SILLocation Loc) {
Type PointerInt8Ty = BoundGenericType::get(PointerDecl,
nullptr,
Int8Decl->getDeclaredType());
Type OptPointerInt8Ty = OptionalType::get(PointerInt8Ty);
PtrPtrInt8Ty = BoundGenericType::get(PointerDecl,
nullptr,
PointerInt8Ty)
OptPointerInt8Ty)
->getCanonicalType();
}
}
Expand Down
Loading

1 comment on commit bc83940

@lattner
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome, congrats Jordan!

Please sign in to comment.