@swift-ci create

> The issue seems to be that [..] gets unconditionally treated as [if there was an explicit initialization with nil].

IMO, by itself, that is very unsurprising behavior. This is something that is guaranteed – Optional properties are default-initialized to nil if one does not spell out a default initial value in the source.

I feel the more surprising thing is: why is the "call _$s3Foo15NothingOptionalV1iSiSgvpfi" still present instead of getting inlined away? Maybe there's an attribute that is being dropped – if the property is marked as @usableFromInline, I think it should be safe to infer @inlinable on the function that computes the initial value, but perhaps we're not doing that.

See SR-11768.

I think Johannes concern is that even if `init` does spell out a default value, the init-to-nil happens anyway. That means the only way to spell a default initial value is to do so at the definition site, which is quite a surprise.

indeed

> I think Johannes concern is that even if `init` does spell out a default value, the init-to-nil happens anyway. That means the only way to spell a default initial value is to do so at the definition site, which is quite a surprise.

I got that. Let me rephrase my point and provide more context for my thought process, at the risk of explaining things that you already know.

If the compiler were not an optimizing compiler, the most simple thing to do would be to do what is doing right now – default initialize based on the definition site (in this case nothing is specified, so it will default initialize using nil), and then perform the assignments as the user wrote out in the init() method.

Notably, this naive codegen does not involve looking at what is written in the init() at all (perhaps contrary to expectation).

Usually, an optimizing compiler would be organized so that it still does the same naive codegen, and then adds extra passes to clean up the IR to something reasonable. In this particular case, since the thing that needs to be cleaned up is a call (you can check that this actually needs to happen using -emit-sil), there are a couple of options:

1. The call gets inlined - then constant propagation + copy propagation + dead code elimination can take care of the rest.
2. Do some interprocedural analysis to conclude that any of the writes in the particular function will be overwritten before further reads, so it is safe to eliminate that call.

Usually, the hammer of choice is inlining. Which is why I pointed out that I was surprised that the call didn't get inlined. To summarize, there are two different perspectives here:

1. That call shouldn't get generated in the first place because the init has a default value for the field – if I understand correctly, this is what Johannes is suggesting.
2. It is fine if the call is generated, but it should get optimized out – this is my point-of-view.

—

As an aside, I'm not sure why this bug report has the LibraryEvolution label on it. The first struct declaration in Foo does not compile with library evolution on. (godbolt)

public struct NothingOptional {
    @usableFromInline var i: Int?
    @inlinable public init() {
        self.i = 0xdeadbef
    }
}

If you add a @frozen, it does compile (godbolt) and the codegen looks good with the constant being written into eax directly in the init().

theindigamer (JIRA User) I agree with all of that. Our guess is that the 'variable initialisation expression' function is not inlined because the Swift compiler deliberately chooses not to because it couldn't do that if this were compiled for library evolution. Now we all agree that we're not using the library evolution mode here so it could be inlined and that is one bug (SR-11768). The other bug (which is what this ticket is about) is that all optional variables actually get double initialised, first to nil and then to the actual value.

SR-XX being SR-11768, which is required reading for this bug I think.

theindigamer (JIRA User) another important clue why all of this is probably related to some library evolution code (that should not be used here) is that if I put the whole program in one module, then the functions all have the assembly that we expect (because now the library evolution stuff is truly not used because we call the inits from the same module instead of across):

                       _$s2t217doNothingOptionalAA0cD0VyF:        // t2.doNothingOptional() -> t2.NothingOptional
0x0000000100001a10         push       rbp
0x0000000100001a11         mov        rbp, rsp
0x0000000100001a14         mov        eax, 0xdeadbef
0x0000000100001a19         xor        edx, edx
0x0000000100001a1b         pop        rbp
0x0000000100001a1c         ret
                        ; endp
0x0000000100001a1d         align      32

                       _$s2t212doNothingIntAA0cD0VyF:        // t2.doNothingInt() -> t2.NothingInt
0x0000000100001a20         push       rbp
0x0000000100001a21         mov        rbp, rsp
0x0000000100001a24         mov        eax, 0xdeadbef
0x0000000100001a29         pop        rbp
0x0000000100001a2a         ret
                        ; endp
0x0000000100001a2b         align      16

                       _$s2t224doNothingOptionalPreventAA0cdE0VyF:        // t2.doNothingOptionalPrevent() -> t2.NothingOptionalPrevent
0x0000000100001a30         push       rbp
0x0000000100001a31         mov        rbp, rsp
0x0000000100001a34         mov        eax, 0xdeadbef
0x0000000100001a39         xor        edx, edx
0x0000000100001a3b         pop        rbp
0x0000000100001a3c         ret
                        ; endp

thanks @Lukasa, forgot to replace the reference at first, edited now.

theindigamer (JIRA User) regarding

> > The issue seems to be that [..] gets unconditionally treated as [if there was an explicit initialization with nil].
>
> IMO, by itself, that is very unsurprising behavior. This is something that is guaranteed – Optional properties are default-initialized to nil if one does not spell out a default initial value in the source.

You may find this "unsurprising" but my expectation would be (and that is coincidentally our workaround of choice for SR-17768) that there is some spelling in the source language makes the language not pre-initialise an optional ivar to nil. I don't really mind what the spelling is but I believe there should be a spelling (as there is for anything that is not Optional<T>).

So far my expectation was that {{Optional<T>}}s are a little special and they only nil auto-initialise iff they are not also set in init. Just like it is the case if they are {{let}}s:

Consider this program

  1 struct OptionalNotInitialisedNotInited {
  2     let foo: Optional<Int>
  3 }
  4 
  5 struct OptionalNotInitialisedButInited {
  6     let foo: Optional<Int>
  7 
  8     init() {
  9         self.foo = 5
 10     }
 11 }
 12 
 13 struct OptionalInitialisedAndInited { 
 14     let foo: Optional<Int> = 6 
 15 
 16     init() { 
 17         self.foo = 5 
 18     } 
 19 }

Lines 1 to 12 are totally valid Swift, OptionalNotInitialisedNotInited (auto-)initialised foo to nil exactly once. OptionalNotInitialisedButInited initialises foo to 5 (and never sets it to 0 before).

Lines 13 to 19 however will not compile because I can't re-initialise foo.'

Thanks for elaborating Johannes and Cory. I think I get the difference between the two Jiras now.

> Now we all agree that we're not using the library evolution mode here so it could be inlined and that is one bug (SR-11768)

Understood. But then my question is: why does this Jira have the LibraryEvolution label? I get why SR-11768 has LibraryEvolution but should the label be removed from this Jira? Or is this Jira (double-initialization) especially bad in the presence of library evolution?

> The other bug (which is what this ticket is about) is that all optional variables actually get double initialised, first to nil and then to the actual value.

I feel like this is not a bug. But that's just my hunch. I should defer to more experienced people here to judge that.

> You may find this "unsurprising" but my expectation would be (and that is coincidentally our workaround of choice for SR-17768) that there is some spelling in the source language makes the language not pre-initialise an optional ivar to nil

That makes a lot of sense. If there were an opt-in way to do this easy (cannot be opt-out because of source breakage), say something like an attribute @_noDefaultInit, which makes Optionals behave like other types and not have this magic default initialization, that would certainly be great 🙂. If this is the desired outcome, then I suggest changing it to a Feature/Enhancement Request instead of a Bug. [Edit: Maybe not, see the end of the message.]

> So far my expectation was that Optional<T> are a little special and they only nil auto-initialise iff they are not also set in init. Just like it is the case if they are {{let}}s

Ah, I see Here's why (I think) it doesn't work the same way for var as it does for let:

1. If the compiler sees a let which lacks a user-defined initializer expression, it knows there will be an error anyways if the field is not initialized in the init(). Hence, it is both safe and beneficial to not perform any default initialization at the beginning. The body of the init() does not need to be inspected to arrive at this conclusion.

2. If the compiler sees a var which lacks a user-defined initializer expression, there are two options. (1) Be naive: default initialize at the beginning, and if there are any writes later, that's still ok (suboptimal from a perf perspective but still safe). This approach doesn't require looking at the body of the init() (2) Be smart: look at the body of the init() to see if there is any initialization going on. Skip the default initialization at the beginning if and only if the field is not initialized. [There are further complications here because the definite initialization analysis happens after SILGen but let's ignore that.] Since approach (2) requires doing more work, it is reasonable to follow approach (1).

All that said and done, I think there is a solution! Don't use the ? sugar, use Optional. That has different behavior for default initialization (made clear in this commit?): 25e4ca9

struct S {
  var x: Optional<Int>
  init() { } // error: self.x is not initialized
} 

struct S {
 var x: Int?
 init() { } // ok: self.x does get default initialization
}

theindigamer (JIRA User) OMG, TIL, thank you so much! I change NIO's uses of var foo: T? to var foo: Optional<T> after confirming that the code is actually better:

                       _$s7TestApp17doNothingOptional3Foo0dE0VyF:        // TestApp.doNothingOptional() -> Foo.NothingOptional
0x0000000100001c00         push       rbp                                       ; CODE XREF=_main+57
0x0000000100001c01         mov        rbp, rsp
0x0000000100001c04         call       _$s3Foo15NothingOptionalV1iSiSgvpfi       ; variable initialization expression of Foo.NothingOptional.i : Swift.Int?
0x0000000100001c09         mov        eax, 0xdeadbef
0x0000000100001c0e         xor        edx, edx
0x0000000100001c10         pop        rbp
0x0000000100001c11         ret

                       _$s7TestApp25doNothingOptionalLongName3Foo0defG0VyF:        // TestApp.doNothingOptionalLongName() -> Foo.NothingOptionalLongName
0x0000000100001c20         push       rbp
0x0000000100001c21         mov        rbp, rsp
0x0000000100001c24         mov        eax, 0xdeadbef
0x0000000100001c29         xor        edx, edx
0x0000000100001c2b         pop        rbp
0x0000000100001c2c         ret

the only difference between doNothingOptional and doNothingOptionalLongName is T? vs Optional<T>.

this is the SwiftNIO PR to get rid of T?: apple/swift-nio#1252

Marking as resolved since using Optional<T> instead of T? seems to have the desired effect. 🙂

@weissi I read your PR and you mention it is a "workaround" – does this not resolve the issue? The other option would be to change the behavior of T? and that can't be done because that would be source-breaking.

Do you still want some kind of special annotation on top of T? that makes it behave like Optional<T>? Something else?

thanks theindigamer (JIRA User). For now, I'm happy with the workaround but I do think that SR-11768 is still a bug, I think with resilience off, we shouldn't need (@frozen) to get the initialisers to inline...

That T? is different to Optional<T> in my mind is unacceptable but I do understand that we can't easily change the behaviour now.

> For now, I'm happy with the workaround but I do think that SR-11768is still a bug, I think with resilience off, we shouldn't need (@frozen) to get the initialisers to inline...

Ok, so this one's resolved, you can continue discussing with Slava for that one. 🙂

> That T? is different to Optional<T> in my mind is unacceptable but I do understand that we can't easily change the behaviour now.

I was sufficiently alarmed about it to make a tweet referencing this discovery https://twitter.com/cutculus/status/1195050506177003521

But yeah, it would be heavily source-breaking, so we can't change this unless we apply an automated refactoring that goes and adds nils in places where people are using T?.