Skip to content

Commit af0c7bd

Browse files
cwillmorcwillmor
committed
Initial implementation of SE-0054 "Abolish IUO Type" (#2322)
This is a squash of the following commits: * [SE-0054] Import function pointer arg, return types, typedefs as optional IUOs are only allowed on function decl arguments and return types, so don't import typedefs or function pointer args or return types as IUO. * [SE-0054] Only allow IUOs in function arg and result type. When validating a TypeRepr, raise a diagnostic if an IUO is found anywhere other thn the top level or as a function parameter or return tpye. * [SE-0054] Disable inference of IUOs by default When considering a constraint of the form '$T1 is convertible to T!', generate potential bindings 'T' and 'T?' for $T1, but not 'T!'. This prevents variables without explicit type information from ending up with IUO type. It also prevents implicit instantiation of functions and types with IUO type arguments. * [SE-0054] Remove the -disable-infer-iuos flag. * Add nonnull annotations to ObjectiveCTests.h in benchmark suite.
1 parent 2ef514e commit af0c7bd

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+178
-320
lines changed

‎benchmark/utils/ObjectiveCTests/ObjectiveCTests.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
#import <Foundation/Foundation.h>
1414

15+
NS_ASSUME_NONNULL_BEGIN
16+
1517
@interface BridgeTester : NSObject {
1618
NSString *myString;
1719
NSArray<NSString *> *myArrayOfStrings;
@@ -24,3 +26,5 @@
2426
- (NSArray<NSString *> *)testToArrayOfStrings;
2527

2628
@end
29+
30+
NS_ASSUME_NONNULL_END

‎include/swift/AST/DiagnosticsSema.def

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,6 +1461,10 @@ NOTE(override_unnecessary_IUO_use_strict,none,
14611461
NOTE(override_unnecessary_IUO_silence,none,
14621462
"add parentheses to silence this warning", ())
14631463

1464+
ERROR(iuo_in_illegal_position,none,
1465+
"implicitly unwrapped optionals are only allowed at top level and as "
1466+
"function results", ())
1467+
14641468
ERROR(override_mutable_covariant_property,none,
14651469
"cannot override mutable property %0 of type %1 with covariant type %2",
14661470
(Identifier, Type, Type))

‎include/swift/Basic/LangOptions.h

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,9 +157,6 @@ namespace swift {
157157
/// Enable the Swift 3 migration via Fix-Its.
158158
bool Swift3Migration = false;
159159

160-
/// Allow IUO types to be inferred for otherwise untyped variables.
161-
bool InferIUOs = true;
162-
163160
/// Sets the target we are building for and updates platform conditions
164161
/// to match.
165162
///

‎include/swift/Option/FrontendOptions.td

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -244,10 +244,6 @@ def swift3_migration :
244244
Flag<["-"], "swift3-migration">,
245245
HelpText<"Enable Fix-It based migration aids for Swift 3">;
246246

247-
def disable_infer_iuos :
248-
Flag<["-"], "disable-infer-iuos">,
249-
HelpText<"Disable inferring IUO type for otherwise unconstrained variables">;
250-
251247
def warn_omit_needless_words :
252248
Flag<["-"], "Womit-needless-words">,
253249
HelpText<"Warn about needless words in names">;

‎lib/ClangImporter/ImportDecl.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1325,13 +1325,15 @@ namespace {
13251325
auto storedUnderlyingType = Impl.importType(
13261326
Decl->getUnderlyingType(), ImportTypeKind::Value,
13271327
isInSystemModule(DC),
1328-
Decl->getUnderlyingType()->isBlockPointerType());
1328+
Decl->getUnderlyingType()->isBlockPointerType(),
1329+
OTK_Optional);
13291330

13301331
// Find a bridged type, which may be different
13311332
auto computedPropertyUnderlyingType = Impl.importType(
13321333
Decl->getUnderlyingType(), ImportTypeKind::Property,
13331334
isInSystemModule(DC),
1334-
Decl->getUnderlyingType()->isBlockPointerType());
1335+
Decl->getUnderlyingType()->isBlockPointerType(),
1336+
OTK_Optional);
13351337

13361338
if (storedUnderlyingType.getCanonicalTypeOrNull() ==
13371339
computedPropertyUnderlyingType.getCanonicalTypeOrNull()) {
@@ -1365,7 +1367,8 @@ namespace {
13651367
SwiftType = Impl.importType(ClangType,
13661368
ImportTypeKind::Typedef,
13671369
isInSystemModule(DC),
1368-
ClangType->isBlockPointerType());
1370+
ClangType->isBlockPointerType(),
1371+
OTK_Optional);
13691372
}
13701373

13711374
if (!SwiftType)

‎lib/ClangImporter/ImportType.cpp

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -456,7 +456,8 @@ namespace {
456456
auto resultTy = Impl.importType(type->getReturnType(),
457457
ImportTypeKind::Result,
458458
AllowNSUIntegerAsInt,
459-
CanFullyBridgeTypes);
459+
CanFullyBridgeTypes,
460+
OTK_Optional);
460461
if (!resultTy)
461462
return Type();
462463

@@ -466,7 +467,8 @@ namespace {
466467
param != paramEnd; ++param) {
467468
auto swiftParamTy = Impl.importType(*param, ImportTypeKind::Parameter,
468469
AllowNSUIntegerAsInt,
469-
CanFullyBridgeTypes);
470+
CanFullyBridgeTypes,
471+
OTK_Optional);
470472
if (!swiftParamTy)
471473
return Type();
472474

@@ -489,7 +491,8 @@ namespace {
489491
auto resultTy = Impl.importType(type->getReturnType(),
490492
ImportTypeKind::Result,
491493
AllowNSUIntegerAsInt,
492-
CanFullyBridgeTypes);
494+
CanFullyBridgeTypes,
495+
OTK_Optional);
493496
if (!resultTy)
494497
return Type();
495498

‎lib/Frontend/CompilerInvocation.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -760,9 +760,6 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
760760
Opts.WarnOmitNeedlessWords = Args.hasArg(OPT_warn_omit_needless_words);
761761
Opts.StripNSPrefix |= Args.hasArg(OPT_enable_strip_ns_prefix);
762762
Opts.InferImportAsMember |= Args.hasArg(OPT_enable_infer_import_as_member);
763-
if (Args.hasArg(OPT_disable_infer_iuos)) {
764-
Opts.InferIUOs = false;
765-
}
766763

767764
Opts.EnableThrowWithoutTry |= Args.hasArg(OPT_enable_throw_without_try);
768765

‎lib/Sema/CSSolver.cpp

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -875,18 +875,15 @@ static PotentialBindings getPotentialBindings(ConstraintSystem &cs,
875875
}
876876
}
877877

878+
// Don't deduce IUO types.
878879
Type alternateType;
879-
ASTContext &ctx = cs.getTypeChecker().Context;
880-
if (!ctx.LangOpts.InferIUOs) {
881-
// Don't deduce IUO types.
882-
if (kind == AllowedBindingKind::Supertypes &&
883-
constraint->getKind() >= ConstraintKind::Conversion &&
884-
constraint->getKind() <= ConstraintKind::OperatorArgumentConversion) {
885-
if (auto objectType =
886-
cs.lookThroughImplicitlyUnwrappedOptionalType(type)) {
887-
type = OptionalType::get(objectType);
888-
alternateType = objectType;
889-
}
880+
if (kind == AllowedBindingKind::Supertypes &&
881+
constraint->getKind() >= ConstraintKind::Conversion &&
882+
constraint->getKind() <= ConstraintKind::OperatorArgumentConversion) {
883+
if (auto objectType =
884+
cs.lookThroughImplicitlyUnwrappedOptionalType(type)) {
885+
type = OptionalType::get(objectType);
886+
alternateType = objectType;
890887
}
891888
}
892889

‎lib/Sema/TypeCheckType.cpp

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1340,6 +1340,64 @@ Type TypeChecker::resolveIdentifierType(
13401340
return result;
13411341
}
13421342

1343+
// Returns true if any illegal IUOs were found. If inference of IUO type is disabled, IUOs may only be specified in the following positions:
1344+
// * outermost type
1345+
// * function param
1346+
// * function return type
1347+
static bool checkForIllegalIUOs(TypeChecker &TC, TypeRepr *Repr,
1348+
TypeResolutionOptions Options) {
1349+
class IllegalIUOWalker : public ASTWalker {
1350+
TypeChecker &TC;
1351+
SmallVector<bool, 4> IUOsAllowed;
1352+
bool FoundIllegalIUO = false;
1353+
1354+
public:
1355+
IllegalIUOWalker(TypeChecker &TC, bool IsGenericParameter)
1356+
: TC(TC)
1357+
, IUOsAllowed{!IsGenericParameter} {}
1358+
1359+
bool walkToTypeReprPre(TypeRepr *T) {
1360+
bool iuoAllowedHere = IUOsAllowed.back();
1361+
1362+
// Raise a diagnostic if we run into a prohibited IUO.
1363+
if (!iuoAllowedHere) {
1364+
if (auto *iuoTypeRepr =
1365+
dyn_cast<ImplicitlyUnwrappedOptionalTypeRepr>(T)) {
1366+
TC.diagnose(iuoTypeRepr->getStartLoc(), diag::iuo_in_illegal_position)
1367+
.fixItReplace(iuoTypeRepr->getExclamationLoc(), "?");
1368+
FoundIllegalIUO = true;
1369+
}
1370+
}
1371+
1372+
bool childIUOsAllowed = false;
1373+
if (iuoAllowedHere) {
1374+
if (auto *tupleTypeRepr = dyn_cast<TupleTypeRepr>(T)) {
1375+
if (tupleTypeRepr->isParenType()) {
1376+
childIUOsAllowed = true;
1377+
}
1378+
} else if (isa<FunctionTypeRepr>(T)) {
1379+
childIUOsAllowed = true;
1380+
} else if (isa<AttributedTypeRepr>(T) || isa<InOutTypeRepr>(T)) {
1381+
childIUOsAllowed = true;
1382+
}
1383+
}
1384+
IUOsAllowed.push_back(childIUOsAllowed);
1385+
return true;
1386+
}
1387+
1388+
bool walkToTypeReprPost(TypeRepr *T) {
1389+
IUOsAllowed.pop_back();
1390+
return true;
1391+
}
1392+
1393+
bool getFoundIllegalIUO() const { return FoundIllegalIUO; }
1394+
};
1395+
1396+
IllegalIUOWalker Walker(TC, Options.contains(TR_GenericSignature));
1397+
Repr->walk(Walker);
1398+
return Walker.getFoundIllegalIUO();
1399+
}
1400+
13431401
bool TypeChecker::validateType(TypeLoc &Loc, DeclContext *DC,
13441402
TypeResolutionOptions options,
13451403
GenericTypeResolver *resolver,
@@ -1351,6 +1409,9 @@ bool TypeChecker::validateType(TypeLoc &Loc, DeclContext *DC,
13511409
return Loc.isError();
13521410

13531411
if (Loc.getType().isNull()) {
1412+
// Raise error if we parse an IUO type in an illegal position.
1413+
checkForIllegalIUOs(*this, Loc.getTypeRepr(), options);
1414+
13541415
auto type = resolveType(Loc.getTypeRepr(), DC, options, resolver,
13551416
unsatisfiedDependency);
13561417
if (!type) {

‎stdlib/private/StdlibUnittest/StdlibUnittest.swift.gyb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -599,7 +599,7 @@ func _printDebuggingAdvice(_ fullTestName: String) {
599599
var invocation = [Process.arguments[0]]
600600
let interpreter = getenv("SWIFT_INTERPRETER")
601601
if interpreter != nil {
602-
if let interpreterCmd = String(validatingUTF8: interpreter) {
602+
if let interpreterCmd = String(validatingUTF8: interpreter!) {
603603
invocation.insert(interpreterCmd, at: 0)
604604
}
605605
}

‎stdlib/private/SwiftPrivateLibcExtras/Subprocess.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ public func spawnChild(_ args: [String])
184184
childArgs.insert(Process.arguments[0], at: 0)
185185
let interpreter = getenv("SWIFT_INTERPRETER")
186186
if interpreter != nil {
187-
if let invocation = String(validatingUTF8: interpreter) {
187+
if let invocation = String(validatingUTF8: interpreter!) {
188188
childArgs.insert(invocation, at: 0)
189189
}
190190
}

‎stdlib/private/SwiftReflectionTest/SwiftReflectionTest.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ internal func getReflectionInfoForImage(atIndex i: UInt32) -> ReflectionInfo? {
112112
let header = unsafeBitCast(_dyld_get_image_header(i),
113113
to: UnsafePointer<MachHeader>.self)
114114

115-
let imageName = _dyld_get_image_name(i)
115+
let imageName = _dyld_get_image_name(i)!
116116
if let fieldmd = getSectionInfo("__swift3_fieldmd", header) {
117117
let assocty = getSectionInfo("__swift3_assocty", header)
118118
let builtin = getSectionInfo("__swift3_builtin", header)
@@ -189,7 +189,7 @@ internal func sendReflectionInfos() {
189189

190190
internal func printErrnoAndExit() {
191191
debugLog("BEGIN \(#function)"); defer { debugLog("END \(#function)") }
192-
let errorCString = strerror(errno)
192+
let errorCString = strerror(errno)!
193193
let message = String(validatingUTF8: errorCString)! + "\n"
194194
let bytes = Array(message.utf8)
195195
fwrite(bytes, 1, bytes.count, stderr)

‎stdlib/public/SDK/ObjectiveC/ObjectiveC.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ extension Selector : CustomStringConvertible {
149149
if name == nil {
150150
return "<NULL>"
151151
}
152-
return String(cString: name)
152+
return String(cString: name!)
153153
}
154154
}
155155

‎stdlib/public/core/FloatingPointParsing.swift.gyb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ extension ${Self} {
6161
_swift_stdlib_strto${cFuncSuffix2[bits]}_clocale(
6262
chars, UnsafeMutablePointer($0))
6363
}
64-
return (result, endPtr == nil ? 0 : UnsafePointer(endPtr) - chars)
64+
return (result, endPtr == nil ? 0 : UnsafePointer(endPtr!) - chars)
6565
}
6666

6767
let (result, n) = text.withCString(parseNTBS)

‎stdlib/public/core/ImplicitlyUnwrappedOptional.swift

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,14 +91,14 @@ extension ImplicitlyUnwrappedOptional : _ObjectiveCBridgeable {
9191

9292
public static func _forceBridgeFromObjectiveC(
9393
_ x: AnyObject,
94-
result: inout Wrapped!?
94+
result: inout ImplicitlyUnwrappedOptional<Wrapped>?
9595
) {
9696
result = Swift._forceBridgeFromObjectiveC(x, Wrapped.self)
9797
}
9898

9999
public static func _conditionallyBridgeFromObjectiveC(
100100
_ x: AnyObject,
101-
result: inout Wrapped!?
101+
result: inout ImplicitlyUnwrappedOptional<Wrapped>?
102102
) -> Bool {
103103
let bridged: Wrapped? =
104104
Swift._conditionallyBridgeFromObjectiveC(x, Wrapped.self)
@@ -115,7 +115,7 @@ extension ImplicitlyUnwrappedOptional : _ObjectiveCBridgeable {
115115

116116
public static func _unconditionallyBridgeFromObjectiveC(_ source: AnyObject?)
117117
-> Wrapped! {
118-
var result: Wrapped!?
118+
var result: ImplicitlyUnwrappedOptional<Wrapped>?
119119
_forceBridgeFromObjectiveC(source!, result: &result)
120120
return result!
121121
}

‎test/1_stdlib/Optional.swift

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -379,20 +379,6 @@ OptionalTests.test("Optional OutputStream") {
379379
expectEqual(String(optNoString), "Optional(main.TestNoString)")
380380
expectEqual(debugPrintStr(optNoString), "Optional(main.TestNoString)")
381381

382-
let iouNoString: TestNoString! = TestNoString()
383-
// IUO directly conforms to CustomStringConvertible.
384-
// Disabled pending SR-164
385-
// expectTrue(iouNoString is CustomStringConvertible)
386-
expectTrue(canGenericCast(iouNoString, CustomStringConvertible.self))
387-
expectFalse(iouNoString is Streamable)
388-
expectFalse(canGenericCast(iouNoString, Streamable.self))
389-
// CustomDebugStringConvertible conformance is a temporary hack.
390-
// Disabled pending SR-164
391-
// expectTrue(iouNoString is CustomDebugStringConvertible)
392-
expectTrue(canGenericCast(iouNoString, CustomDebugStringConvertible.self))
393-
expectEqual(String(iouNoString), "main.TestNoString")
394-
expectEqual(debugPrintStr(iouNoString), "main.TestNoString")
395-
396382
let optString: TestString? = TestString()
397383
expectTrue(optString is CustomStringConvertible)
398384
expectTrue(canGenericCast(optString, CustomStringConvertible.self))
@@ -402,18 +388,6 @@ OptionalTests.test("Optional OutputStream") {
402388
expectEqual(String(optString), "Optional(XString)")
403389
expectEqual(debugPrintStr(optString), "Optional(XString)")
404390

405-
let iouString: TestString! = TestString()
406-
expectTrue(iouString is CustomStringConvertible)
407-
expectTrue(canGenericCast(iouString, CustomStringConvertible.self))
408-
// CustomDebugStringConvertible conformance is a temporary hack.
409-
expectTrue(iouString is CustomDebugStringConvertible)
410-
expectTrue(canGenericCast(iouString, CustomDebugStringConvertible.self))
411-
expectEqual(String(iouString), "AString")
412-
// FIXME: Ideally the debug output would be "XString", but a reasonable
413-
// implementation of that behavior requires conditional conformance.
414-
// (directly invoking debugPrint(Any) already works correctly).
415-
expectEqual(debugPrintStr(iouString), "AString")
416-
417391
let optStream: TestStream? = TestStream()
418392
expectTrue(optStream is Streamable)
419393
expectTrue(canGenericCast(optStream, Streamable.self))

‎test/1_stdlib/PrintString.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ PrintTests.test("Printable") {
3737
}
3838

3939
PrintTests.test("Printable") {
40-
expectPrinted("meow", String!("meow"))
4140
expectPrinted("Optional(\"meow\")", String?("meow"))
4241
}
4342

‎test/ClangModules/Inputs/sdk-bridging-header.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,8 @@
33
@class NSArray;
44

55
@interface Predicate : NSObject
6-
+ (Predicate *)truePredicate;
7-
+ (Predicate *)not;
8-
+ (Predicate *)and:(NSArray *)subpredicates;
9-
+ (Predicate *)or:(NSArray *)subpredicates;
10-
@end
6+
+ (nonnull Predicate *)truePredicate;
7+
+ (nonnull Predicate *)not;
8+
+ (nonnull Predicate *)and:(nonnull NSArray *)subpredicates;
9+
+ (nonnull Predicate *)or:(nonnull NSArray *)subpredicates;
10+
@end

‎test/ClangModules/blocks_parse.swift

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,23 @@ func testNoEscape(f: @noescape @convention(block) () -> Void, nsStr: NSString,
2323
// rdar://problem/19818617
2424
nsStr.enumerateLines(fStr) // okay due to @noescape
2525

26-
_ = nsStr.enumerateLines as Int // expected-error{{cannot convert value of type '(@noescape (String!) -> Void) -> Void' to type 'Int' in coercion}}
26+
_ = nsStr.enumerateLines as Int // expected-error{{cannot convert value of type '(@noescape (String) -> Void) -> Void' to type 'Int' in coercion}}
2727
}
2828

2929
func checkTypeImpl<T>(_ a: inout T, _: T.Type) {}
3030
do {
31-
var block = blockWithoutNullability()
32-
checkTypeImpl(&block, ImplicitlyUnwrappedOptional<dispatch_block_t>.self)
31+
var blockOpt = blockWithoutNullability()
32+
checkTypeImpl(&blockOpt, Optional<dispatch_block_t>.self)
33+
var block: dispatch_block_t = blockWithoutNullability()
3334
}
3435
do {
3536
var block = blockWithNonnull()
3637
checkTypeImpl(&block, dispatch_block_t.self)
3738
}
3839
do {
39-
var block = blockWithNullUnspecified()
40-
checkTypeImpl(&block, ImplicitlyUnwrappedOptional<dispatch_block_t>.self)
40+
var blockOpt = blockWithNullUnspecified()
41+
checkTypeImpl(&blockOpt, Optional<dispatch_block_t>.self)
42+
var block: dispatch_block_t = blockWithNullUnspecified()
4143
}
4244
do {
4345
var block = blockWithNullable()

0 commit comments

Comments
 (0)