Skip to content

Commit 1f12e72

Browse files
committed
DI: Suggest "self.init()" for extension initializers of C structs
...unless the struct contains a field that cannot be zero-initialized, such as a non-nullable pointer. This suggestion is only made for C structs because 'init()' may not be the right choice for other structs.
1 parent 1598a21 commit 1f12e72

File tree

4 files changed

+89
-6
lines changed

4 files changed

+89
-6
lines changed

include/swift/AST/DiagnosticsSIL.def

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,11 @@ ERROR(mutating_protocol_witness_method_on_let_constant,none,
217217

218218
WARNING(designated_init_in_cross_module_extension,none,
219219
"initializer for struct %0 must use \"self.init(...)\" or \"self = ...\""
220-
"%select{| on all paths}1 because it is not in module %2",
221-
(Type, bool, Identifier))
220+
"%select{| on all paths}1 because "
221+
"%select{it is not in module %2|the struct was imported from C}3",
222+
(Type, bool, Identifier, bool))
223+
NOTE(designated_init_c_struct_fix,none,
224+
"use \"self.init()\" to initialize the struct with zero values", ())
222225

223226

224227
// Control flow diagnostics.

lib/SILOptimizer/Mandatory/DefiniteInitialization.cpp

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#include "swift/AST/DiagnosticEngine.h"
1616
#include "swift/AST/DiagnosticsSIL.h"
1717
#include "swift/AST/Expr.h"
18+
#include "swift/ClangImporter/ClangModule.h"
1819
#include "swift/SIL/InstructionUtils.h"
1920
#include "swift/SIL/SILArgument.h"
2021
#include "swift/SIL/SILBuilder.h"
@@ -46,10 +47,13 @@ llvm::cl::opt<bool> TriggerUnreachableOnFailure(
4647
STATISTIC(NumAssignRewritten, "Number of assigns rewritten");
4748

4849
template<typename ...ArgTypes>
49-
static void diagnose(SILModule &M, SILLocation loc, ArgTypes... args) {
50-
M.getASTContext().Diags.diagnose(loc.getSourceLoc(), Diagnostic(args...));
50+
static InFlightDiagnostic diagnose(SILModule &M, SILLocation loc,
51+
ArgTypes... args) {
52+
auto diag = M.getASTContext().Diags.diagnose(loc.getSourceLoc(),
53+
Diagnostic(args...));
5154
if (TriggerUnreachableOnFailure)
5255
llvm_unreachable("Triggering standard assertion failure routine");
56+
return diag;
5357
}
5458

5559
enum class PartialInitializationKind {
@@ -1041,10 +1045,30 @@ void LifetimeChecker::handleStoreUse(unsigned UseID) {
10411045
else
10421046
selfTy = TheMemory.getType();
10431047

1048+
StructDecl *theStruct = selfTy->getStructOrBoundGenericStruct();
1049+
assert(theStruct);
1050+
10441051
diagnose(Module, Use.Inst->getLoc(),
10451052
diag::designated_init_in_cross_module_extension,
10461053
selfTy, !isFullyUninitialized,
1047-
selfTy->getAnyNominal()->getParentModule()->getName());
1054+
theStruct->getParentModule()->getName(),
1055+
theStruct->hasClangNode());
1056+
1057+
// Special case: most (but not all) C structs have a zeroing no-argument
1058+
// initializer. If this struct is such a type, suggest inserting
1059+
// "self.init()".
1060+
if (theStruct->hasClangNode()) {
1061+
ASTContext &ctx = Module.getASTContext();
1062+
DeclName noArgInit(ctx, ctx.Id_init, ArrayRef<Identifier>());
1063+
auto lookupResults = theStruct->lookupDirect(noArgInit);
1064+
if (lookupResults.size() == 1 &&
1065+
lookupResults.front()->getDeclContext() == theStruct) {
1066+
diagnose(Module, Use.Inst->getLoc(),
1067+
diag::designated_init_c_struct_fix)
1068+
.fixItInsert(Use.Inst->getLoc().getStartSourceLoc(),
1069+
"self.init()\n");
1070+
}
1071+
}
10481072

10491073
// Don't emit more than one of these diagnostics per initializer.
10501074
WantsCrossModuleStructInitializerDiagnostic = false;
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
struct CPoint {
2+
double x, y;
3+
};
4+
5+
struct NonnullWrapper {
6+
void * _Nonnull ptr;
7+
};

test/SILOptimizer/definite_init_cross_module_swift4.swift

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// RUN: %empty-directory(%t)
22
// RUN: %target-swift-frontend -emit-module -emit-module-path=%t/OtherModule.swiftmodule %S/Inputs/definite_init_cross_module_swift4/OtherModule.swift
3-
// RUN: %target-swift-frontend -emit-sil -verify -I %t -swift-version 4 %s > /dev/null
3+
// RUN: %target-swift-frontend -emit-sil -verify -I %t -swift-version 4 %s > /dev/null -import-objc-header %S/Inputs/definite_init_cross_module_swift4/BridgingHeader.h
44

55
import OtherModule
66

@@ -172,3 +172,52 @@ extension MyGenericPoint {
172172
self.y = myY
173173
}
174174
}
175+
176+
extension CPoint {
177+
init(xx: Double, yy: Double) {
178+
self.x = xx // expected-warning {{initializer for struct 'CPoint' must use "self.init(...)" or "self = ..." because the struct was imported from C}} expected-note {{use "self.init()" to initialize the struct with zero values}} {{5-5=self.init()\n}}
179+
self.y = yy
180+
}
181+
182+
init(xxx: Double, yyy: Double) {
183+
// This is OK
184+
self.init(x: xxx, y: yyy)
185+
}
186+
187+
init(other: CPoint) {
188+
// This is OK
189+
self = other
190+
}
191+
192+
init(other: CPoint, x: Double) {
193+
// This is OK
194+
self = other
195+
self.x = x
196+
}
197+
198+
init(other: CPoint, xx: Double) {
199+
self.x = xx // expected-warning {{initializer for struct 'CPoint' must use "self.init(...)" or "self = ..." because the struct was imported from C}} expected-note {{use "self.init()" to initialize the struct with zero values}} {{5-5=self.init()\n}}
200+
self = other
201+
}
202+
203+
init(other: CPoint, x: Double, cond: Bool) {
204+
// This is OK
205+
self = other
206+
if cond { self.x = x }
207+
}
208+
209+
init(other: CPoint, xx: Double, cond: Bool) {
210+
if cond { self = other }
211+
self.x = xx // expected-warning {{initializer for struct 'CPoint' must use "self.init(...)" or "self = ..." on all paths because the struct was imported from C}} expected-note {{use "self.init()" to initialize the struct with zero values}} {{5-5=self.init()\n}}
212+
self.y = 0
213+
}
214+
}
215+
216+
217+
extension NonnullWrapper {
218+
init(p: UnsafeMutableRawPointer) {
219+
self.ptr = p // expected-warning {{initializer for struct 'NonnullWrapper' must use "self.init(...)" or "self = ..." because the struct was imported from C}}
220+
// No suggestion for "self.init()" because this struct does not support a
221+
// zeroing initializer.
222+
}
223+
}

0 commit comments

Comments
 (0)