Skip to content

Commit e1ef962

Browse files
committed
improve the DI diagnostic for uses of self in a non-delegating protocol initializer,
which is most likely because someone forgot to chain to self.init completely. Swift SVN r30187
1 parent b0bcad7 commit e1ef962

File tree

4 files changed

+95
-46
lines changed

4 files changed

+95
-46
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@ ERROR(self_use_before_fully_init,sil_analysis,none,
117117
"super.init initializes self}2", (Identifier, bool, bool))
118118
ERROR(use_of_self_before_fully_init,sil_analysis,none,
119119
"'self' used before all stored properties are initialized", ())
120+
ERROR(use_of_self_before_fully_init_protocol,sil_analysis,none,
121+
"'self' used before chaining to another self.init requirement", ())
122+
120123

121124
NOTE(stored_property_not_initialized,sil_analysis,none,
122125
"'%0' not initialized", (StringRef))

lib/SILPasses/DIMemoryUseCollector.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -711,9 +711,10 @@ void ElementUseCollector::collectUses(SILValue Pointer, unsigned BaseEltNo) {
711711
// individual sub-member is passed as inout, then we model that as an
712712
// inout use.
713713
auto Kind = DIUseKind::InOutUse;
714-
if (TheMemory.isStructInitSelf() && Pointer == TheMemory.getAddress())
714+
if ((TheMemory.isStructInitSelf() || TheMemory.isProtocolInitSelf()) &&
715+
Pointer == TheMemory.getAddress())
715716
Kind = DIUseKind::Escape;
716-
717+
717718
addElementUses(BaseEltNo, PointeeType, User, Kind);
718719
continue;
719720
}

lib/SILPasses/DefiniteInitialization.cpp

Lines changed: 72 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -397,9 +397,11 @@ namespace {
397397

398398
void handleStoreUse(unsigned UseID);
399399
void handleInOutUse(const DIMemoryUse &Use);
400+
void handleEscapeUse(const DIMemoryUse &Use);
400401

401402
void handleLoadUseFailure(const DIMemoryUse &InstInfo,
402403
bool IsSuperInitComplete);
404+
403405
void handleSuperInitUse(const DIMemoryUse &InstInfo);
404406
void handleSelfInitUse(DIMemoryUse &InstInfo);
405407
void updateInstructionForInitState(DIMemoryUse &InstInfo);
@@ -519,6 +521,13 @@ void LifetimeChecker::noteUninitializedMembers(const DIMemoryUse &Use) {
519521
assert(TheMemory.isAnyInitSelf() && !TheMemory.isDelegatingInit() &&
520522
"Not an designated initializer");
521523

524+
// Root protocol initializers (ones that reassign to self, not delegating to
525+
// self.init) have no members to initialize and self itself has already been
526+
// reported to be uninit in the primary diagnostic.
527+
if (TheMemory.isProtocolInitSelf())
528+
return;
529+
530+
522531
// Determine which members, specifically are uninitialized.
523532
AvailabilitySet Liveness =
524533
getLivenessAtInst(Use.Inst, Use.FirstElement, Use.NumElements);
@@ -666,47 +675,8 @@ void LifetimeChecker::doIt() {
666675
case DIUseKind::InOutUse:
667676
handleInOutUse(Use);
668677
break;
669-
670678
case DIUseKind::Escape:
671-
if (!isInitializedAtUse(Use)) {
672-
Diag<StringRef> DiagMessage;
673-
674-
// This is a use of an uninitialized value. Emit a diagnostic.
675-
if (TheMemory.isDelegatingInit()) {
676-
DiagMessage = diag::self_use_before_init_in_delegatinginit;
677-
678-
// If this is a load with a single user that is a return, then this is
679-
// a return before self.init. Emit a specific diagnostic.
680-
if (auto *LI = dyn_cast<LoadInst>(Inst))
681-
if (LI->hasOneUse() &&
682-
isa<ReturnInst>((*LI->use_begin())->getUser())) {
683-
if (shouldEmitError(Inst))
684-
diagnose(Module, Inst->getLoc(),
685-
diag::return_from_init_without_self_init);
686-
break;
687-
}
688-
if (isa<ReturnInst>(Inst)) {
689-
if (shouldEmitError(Inst))
690-
diagnose(Module, Inst->getLoc(),
691-
diag::return_from_init_without_self_init);
692-
break;
693-
}
694-
} else if (isa<ApplyInst>(Inst) && TheMemory.isStructInitSelf()) {
695-
if (shouldEmitError(Inst)) {
696-
diagnose(Module, Inst->getLoc(),
697-
diag::use_of_self_before_fully_init);
698-
noteUninitializedMembers(Use);
699-
}
700-
break;
701-
} else if (isa<MarkFunctionEscapeInst>(Inst))
702-
DiagMessage = diag::global_variable_function_use_uninit;
703-
else if (isa<AddressToPointerInst>(Inst))
704-
DiagMessage = diag::variable_addrtaken_before_initialized;
705-
else
706-
DiagMessage = diag::variable_escape_before_initialized;
707-
708-
diagnoseInitError(Use, DiagMessage);
709-
}
679+
handleEscapeUse(Use);
710680
break;
711681
case DIUseKind::SuperInit:
712682
handleSuperInitUse(Use);
@@ -739,6 +709,7 @@ void LifetimeChecker::doIt() {
739709
handleConditionalDestroys(ControlVariable);
740710
}
741711

712+
742713
void LifetimeChecker::handleStoreUse(unsigned UseID) {
743714
DIMemoryUse &InstInfo = Uses[UseID];
744715

@@ -912,6 +883,60 @@ void LifetimeChecker::handleInOutUse(const DIMemoryUse &Use) {
912883
}
913884
}
914885

886+
void LifetimeChecker::handleEscapeUse(const DIMemoryUse &Use) {
887+
// The value must be fully initialized at all escape points. If not, diagnose
888+
// the error.
889+
if (isInitializedAtUse(Use))
890+
return;
891+
892+
auto Inst = Use.Inst;
893+
894+
// This is a use of an uninitialized value. Emit a diagnostic.
895+
if (TheMemory.isDelegatingInit()) {
896+
// If this is a load with a single user that is a return, then this is
897+
// a return before self.init. Emit a specific diagnostic.
898+
if (auto *LI = dyn_cast<LoadInst>(Inst))
899+
if (LI->hasOneUse() &&
900+
isa<ReturnInst>((*LI->use_begin())->getUser())) {
901+
if (!shouldEmitError(Inst)) return;
902+
diagnose(Module, Inst->getLoc(),
903+
diag::return_from_init_without_self_init);
904+
return;
905+
}
906+
if (isa<ReturnInst>(Inst)) {
907+
if (!shouldEmitError(Inst)) return;
908+
diagnose(Module, Inst->getLoc(),
909+
diag::return_from_init_without_self_init);
910+
return;
911+
}
912+
913+
return diagnoseInitError(Use, diag::self_use_before_init_in_delegatinginit);
914+
}
915+
916+
if (isa<ApplyInst>(Inst) && TheMemory.isAnyInitSelf() &&
917+
!TheMemory.isClassInitSelf()) {
918+
if (!shouldEmitError(Inst)) return;
919+
920+
auto diagID = diag::use_of_self_before_fully_init;
921+
if (TheMemory.isProtocolInitSelf())
922+
diagID = diag::use_of_self_before_fully_init_protocol;
923+
924+
diagnose(Module, Inst->getLoc(), diagID);
925+
noteUninitializedMembers(Use);
926+
return;
927+
}
928+
929+
Diag<StringRef> DiagMessage;
930+
if (isa<MarkFunctionEscapeInst>(Inst))
931+
DiagMessage = diag::global_variable_function_use_uninit;
932+
else if (isa<AddressToPointerInst>(Inst))
933+
DiagMessage = diag::variable_addrtaken_before_initialized;
934+
else
935+
DiagMessage = diag::variable_escape_before_initialized;
936+
937+
diagnoseInitError(Use, DiagMessage);
938+
}
939+
915940

916941
/// Failable enum initializer produce a CFG for the return that looks like this,
917942
/// where the load is the use of 'self'. Detect this pattern so we can consider
@@ -1170,12 +1195,16 @@ void LifetimeChecker::handleLoadUseFailure(const DIMemoryUse &Use,
11701195
return;
11711196
}
11721197

1173-
// If this is a load of self in a struct/enum initializer, then it must be a
1174-
// use of 'self' before all the stored properties are set up.
1198+
// If this is a load of self in a struct/enum/protocol initializer, then it
1199+
// must be a use of 'self' before all the stored properties are set up.
11751200
if (isa<LoadInst>(Inst) && TheMemory.isAnyInitSelf() &&
11761201
!TheMemory.isClassInitSelf()) {
11771202
if (!shouldEmitError(Inst)) return;
1178-
diagnose(Module, Inst->getLoc(), diag::use_of_self_before_fully_init);
1203+
1204+
auto diagID = diag::use_of_self_before_fully_init;
1205+
if (TheMemory.isProtocolInitSelf())
1206+
diagID = diag::use_of_self_before_fully_init_protocol;
1207+
diagnose(Module, Inst->getLoc(), diagID);
11791208
noteUninitializedMembers(Use);
11801209
return;
11811210
}

test/SILPasses/definite_init_diagnostics.swift

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1275,6 +1275,8 @@ class DerivedThrowingInitializer : ThrowingInitializer {
12751275
protocol ProtocolInitTest {
12761276
init()
12771277
init(a : Int)
1278+
1279+
var i: Int { get set }
12781280
}
12791281

12801282
extension ProtocolInitTest {
@@ -1284,6 +1286,20 @@ extension ProtocolInitTest {
12841286
init(b : Float) {
12851287
self.init(a: 42) // ok
12861288
}
1287-
}
12881289

1290+
init(test1 ii: Int) {
1291+
i = ii // expected-error {{use of 'self' in delegating initializer before self.init is called}}
1292+
self.init()
1293+
}
1294+
1295+
init(test2 ii: Int) {
1296+
self = unsafeBitCast(0, Self.self)
1297+
i = ii
1298+
}
1299+
1300+
init(test3 ii: Int) {
1301+
i = ii // expected-error {{'self' used before chaining to another self.init requirement}}
1302+
self = unsafeBitCast(0, Self.self)
1303+
}
1304+
}
12891305

0 commit comments

Comments
 (0)