Skip to content

DI fixes for resilience #12386

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.

Sign up for GitHub

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

Merged
merged 13 commits into from
Oct 14, 2017
Merged

DI fixes for resilience #12386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
e515563
DI: Consolidate and clean up some diagnostics
slavapestov Oct 12, 2017
5ffbd28
DI: Fix diagnostic for return from address-only initializer without c…
slavapestov Oct 12, 2017
9a7a1de
DI: Look through begin_access/end_access for assignment to self
slavapestov Oct 12, 2017
c3cf4ac
DI: For value types, handle self.init like an assignment instead of v…
slavapestov Oct 12, 2017
0c16aed
DI: All protocol extension initializers should be delegating
slavapestov Oct 13, 2017
b5eeae7
DI: All enum initializers should be delegating
slavapestov Oct 13, 2017
065cbd8
AST: Struct initializers that assign to self can now be resilient
slavapestov Oct 13, 2017
94d4b5a
DI: Small cleanup now that SelfInit is only used for classes and clas…
slavapestov Oct 13, 2017
ff34f3b
DI: Remove shouldIgnoreClassMethodUseError()
slavapestov Oct 14, 2017
421797c
DI: Fold handleSuperInitUse() into handleSelfInitUse()
slavapestov Oct 14, 2017
1bcfc50
DI: Kill off collectBorrowedSuperUses()
slavapestov Oct 14, 2017
1feaf9e
DI: Kill off checkClassSelfUpcastUsedBySuperInit()
slavapestov Oct 14, 2017
d8153b3
DI: Correctly flag escaping uses of 'self' when 'self' is passed as a…
slavapestov Oct 14, 2017
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
25 changes: 12 additions & 13 deletions include/swift/AST/DiagnosticsSIL.def
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,36 +151,35 @@ ERROR(ivar_not_initialized_at_implicit_superinit,none,
(StringRef, bool))

ERROR(self_use_before_fully_init,none,
"use of 'self' in %select{method call|property access}1 %0 before "
"'self' used in %select{method call|property access}1 %0 before "
"%select{all stored properties are initialized|"
"super.init initializes self|"
"self.init initializes self}2", (DeclBaseName, bool, unsigned))
"'super.init' call|"
"'self.init' call}2", (DeclBaseName, bool, unsigned))
ERROR(use_of_self_before_fully_init,none,
"'self' used before all stored properties are initialized", ())
ERROR(use_of_self_before_fully_init_protocol,none,
"'self' used before chaining to another self.init requirement", ())


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

ERROR(selfinit_multiple_times,none,
"%select{super|self}0.init called multiple times in initializer",
"'%select{super|self}0.init' called multiple times in initializer",
(unsigned))
ERROR(superselfinit_not_called_before_return,none,
"%select{super|self}0.init isn't called on all paths before returning "
"'%select{super|self}0.init' isn't called on all paths before returning "
"from initializer", (unsigned))
ERROR(self_before_superselfinit,none,
"'self' used before %select{super|self}0.init call",
(unsigned))
ERROR(self_before_superinit,none,
"'self' used before 'super.init' call", ())
ERROR(self_before_selfinit,none,
"'self' used before 'self.init' call", ())
ERROR(self_before_selfinit_value_type,none,
"'self' used before 'self.init' call or assignment to 'self'", ())
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: why have three separate messages here, when it's still a choice above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the choice was keyed purely on whether its a delegating init or not, but I also want to distinguish the value type case so that diagnostics say that an assignment to self is OK too. I'll probably rework the other diagnostics when I do the fix for the return location analysis.

If you think they should still be a single diagnostic with a %select, I don't mind doing that. It seemed a bit silly to define a new enum that's only used in one place, but if I see other diagnostics get too repetitive I can do it.

ERROR(self_inside_catch_superselfinit,none,
"'self' used inside 'catch' block reachable from "
"%select{super|self}0.init call",
(unsigned))
ERROR(return_from_init_without_initing_self,none,
"return from enum initializer method without storing to 'self'", ())
ERROR(return_from_protocol_init_without_initing_self,none,
"protocol extension initializer never chained to 'self.init'", ())
"return from initializer before 'self.init' call or assignment to 'self'", ())
ERROR(return_from_init_without_initing_stored_properties,none,
"return from initializer without initializing all"
" stored properties", ())
Expand Down
23 changes: 23 additions & 0 deletions lib/AST/Decl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5306,6 +5306,29 @@ ConstructorDecl::getDelegatingOrChainedInitKind(DiagnosticEngine *diags,
// get the kind out of the finder.
auto Kind = finder.Kind;

auto *NTD = getDeclContext()->getAsNominalTypeOrNominalTypeExtensionContext();

// Protocol extension and enum initializers are always delegating.
if (Kind == BodyInitKind::None) {
if (isa<ProtocolDecl>(NTD) || isa<EnumDecl>(NTD)) {
Kind = BodyInitKind::Delegating;
}
}

// Struct initializers that cannot see the layout of the struct type are
// always delegating. This occurs if the struct type is not fixed layout,
// and the constructor is either inlinable or defined in another module.
//
// FIXME: Figure out the right condition to use here that does not depend
// on the -enable-resilience flag, and make it conditional on
// -swift-version 5 instead, once the "disallow memberwise cross-module
// initializer" proposal lands.
if (Kind == BodyInitKind::None) {
if (isa<StructDecl>(NTD) &&
!NTD->hasFixedLayout(getParentModule(), getResilienceExpansion())) {
Kind = BodyInitKind::Delegating;
}
}

// If we didn't find any delegating or chained initializers, check whether
// the initializer was explicitly marked 'convenience'.
Expand Down
Loading