Comment by Hassan ElDesouky (JIRA)

pilky (JIRA User), @xedin

1. Is this TypeCheckStmt.cpp#L500 the right place in which I should diagnose this? I tried adding an if statement but it didn't work, so I guess there's more to it than just checking if `ctor->isFailable()`
2. Will I need to remove the old incorrect error message and if so how?

hassaneldesouky (JIRA User) As far as I could see the incorrect error is already removed on master, and this is compiling correctly, which is not the expected behavior I think ...
So what you could try to do is before TypeCheckStmt.cpp#L454 and a check like if is a constructor decl and failable diagnose it. Maybe provide a fix to add `nil` too 🙂

Comment by Hassan ElDesouky (JIRA)

Thank you @LucianoPAlmeida I realized after posting the comment that I have to do it before TypeCheckStmt.cpp#L454.

No problem! But one thing I just noticed is that this code should compile because when a result stmt has no result it means that it is a `return;` so is valid.
So if you can confirm that this example compiles on master, it is fixed I think 🙂

Comment by Hassan ElDesouky (JIRA)

PR: #30707

pilky (JIRA User) Although this message may seem confusing is it the expected behavior because you can return; early on failable initializers but all members should be initialized in all paths, and that just doesn't happen in the guard early return of your example. So what the error is trying to say is that you should initialize label member before return; on the guard statement.

guard let actualLabel = label else {
   self.label = ''" // Label should be initialized before return
   return // It's allowed, but don't fail the init just return early 
}
self.label = actualLabel

Maybe it should say something like `self.label should be initialized in all branches` ... but I'm not sure
cc @slavapestov

Comment by Martin Pilkington (JIRA)

@LucianoPAlmeida The case I gave is just the simplest case to reproduce the issue. Often you will have multiple guard statements making it easier to mistype, and make having to initialise all the other properties more complex. There is also the problem that the reason for having a failable initialiser is you don't want the properties to be set and a value to be returned as it would be in an invalid state.

While the error is technically correct, ideally all compiler errors should point to and explain the actual problem. Firstly, for any initialiser I would expect the error to be on the line with the return statement of the branch where one or more properties aren't set rather than the method definition. And in the case of a failable initialiser I would expect the primary error/fix-it to be that that the user probably wanted to type return nil rather than a plain return, as that is the more likely error. It's worth noting that once all properties are set in a failable initialiser then a plain return would not be an error (it could still be a bug in the user's code if they actually wanted to return nil, but not one the compiler should be expected to detect).