[SILGen] Insert an unreachable if a function declaration's parameter is uninhabited

[theblixguy]

This PR adds a change to SILGenProlog (and SILGenStmt as well) to create an unreachable at the start of a function if any of the function's parameters are uninhabited.

Resolves SR-8434 and SR-8435

[mdiep]

It can be useful to declare a function that takes an uninhabited type. (To fulfill a protocol requirement, e.g.)

I envisioned a warning if you supplied a body for such a function—the compiler should let you omit a body altogether (https://bugs.swift.org/browse/SR-8434).

i.e. I think this should be valid code:

// Even though this returns an `Int`, it doesn't have a `return`. It doesn't need one: it's unreachable.
func f(_: Never) -> Int {}

But I think should get a warning if you write any code in the function body:

func f(_: Never) -> Int {
  return 4 // warning: `f` takes an uninhabited parameter. It's code will never be executed.
}

[theblixguy]

i.e. I think this should be valid code:

// Even though this returns an `Int`, it doesn't have a `return`. It doesn't need one: it's unreachable.
func f(_: Never) -> Int {}

But I think should get a warning if you write any code in the function body:

func f(_: Never) -> Int {
  return 4 // warning: `f` takes an uninhabited parameter. It's code will never be executed.
}

Interesting, although I am not too sure about it. It seems a bit strange for a function with a return type to not have a return statement 🤔

[mdiep]

It seems a bit strange for a function with a return type to not have a return statement

It's also strange to write a body for a function that can never be executed. 🙂

But I definitely don't think it should be a warning to declare a function that takes an uninhabited parameter: that can be useful.

[jrose-apple]

As Matt points out, there are still times where you have to write such a function today, even if it doesn't do anything.

protocol ContrivedExample {
  associatedtype Assoc
  func doSomething(to value: Assoc)
  func doSomethingElse()
}

struct Impl: ContrivedExample {
  typealias Assoc = Never
  func doSomething(to value: Never) { /*required by protocol*/ }
  func doSomethingElse() { print("ok") }
}

So I don't think we can take this as is.

[theblixguy]

As Matt points out, there are still times where you have to write such a function today, even if it doesn't do anything.

protocol ContrivedExample {
  associatedtype Assoc
  func doSomething(to value: Assoc)
  func doSomethingElse()
}

struct Impl: ContrivedExample {
  typealias Assoc = Never
  func doSomething(to value: Never) { /*required by protocol*/ }
  func doSomethingElse() { print("ok") }
}

So I don't think we can take this as is.

I see. So the warning should only be emitted if the body has a return statement?

[jrose-apple]

I kind of regret adding the StarterBug tag now. The point of this task is really to make it so that

func test(x: Never) -> Int {}

compiles, by generating an unreachable at the very start of the function body in SIL. That would automatically result in the diagnostic Matt is talking about getting emitted. I'm not the one to advise how to do that, being not so familiar with SILGen, but @slavapestov might have a suggestion.

[theblixguy]

@jrose-apple Ah! I was wondering if we would need to tweak the type checker as well because otherwise it would complain about a missing return statement.

[jrose-apple]

I think that error comes from SIL-level diagnostics, but I could be wrong.

[theblixguy]

Hmm, you're right, there's a diagnoseMissingReturn and diagnoseUnreachable function in Dataflow Diagnostics which deals with a missing return statement.

It seems maybe we can tweak SILGenEpilog and/or SILGenFunction to insert an unreachable if a function declaration contains an uninhabited parameter type. I'll need to take a deeper look at these classes to understand how everything works. Any suggestions @slavapestov?

[Azoy]

You don’t want SILGenEpilog because you want to insert the unreachable as the first instruction for the function body. You may be able to do this in SILGenProlog however. I would take a look at SILGenFunction::emitProlog unless Slava thinks there is a better way to do this.

[theblixguy]

Hmm, I gave it a shot but it doesn't seem to emit an unreachable.

[theblixguy]

Nevermind, I was running the swift installed on my Mac and not the one built by Ninja 🤦‍♂️ Seems to work!

[theblixguy]

At the moment, I am getting a segfault when invoking the frontend with -emit-silgen:

// test(x:)
sil hidden @$s5test14test1xSis5NeverO_tF : $@convention(thin) (Never) -> Int {
// %0                                             // user: Stack dump:
0.	Program arguments: ./swift -frontend -emit-silgen /Users/suyashsrijan/Desktop/test1.swift
0  swift                    0x000000010a412b98 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
1  swift                    0x000000010a411b68 llvm::sys::RunSignalHandlers() + 248
2  swift                    0x000000010a4131b2 SignalHandler(int) + 258
3  libsystem_platform.dylib 0x00007fff78424b3d _sigtramp + 29
4  libsystem_platform.dylib 0x00007fe17782fa98 _sigtramp + 4282429304
5  swift                    0x00000001073e6299 (anonymous namespace)::SILPrinter::print(swift::SILBasicBlock const*) + 473
6  swift                    0x00000001073e8632 swift::SILFunction::print(swift::SILPrintContext&) const + 6402
7  swift                    0x00000001073eaacb swift::SILModule::print(swift::SILPrintContext&, swift::ModuleDecl*, bool) const + 2363
8  swift                    0x000000010655a2a2 writeSIL(swift::SILModule&, swift::PrimarySpecificPaths const&, swift::CompilerInstance&, swift::CompilerInvocation&) + 194
9  swift                    0x00000001065534bb performCompile(swift::CompilerInstance&, swift::CompilerInvocation&, llvm::ArrayRef<char const*>, int&, swift::FrontendObserver*, swift::UnifiedStatsReporter*) + 10155
10 swift                    0x000000010654fc7d swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) + 3021
11 swift                    0x0000000106505f0e main + 686
12 libdyld.dylib            0x00007fff7823b08d start + 1
Segmentation fault: 11

[jrose-apple]

I'm not a SIL expert, but assuming you're building with assertions but didn't get one, that sounds like the SILPrinter is trying to print a null value for something.

[theblixguy]

@jrose-apple I think so, seems like the basic block is null when SILPrinter is trying to print it. The frontend successfully emits the warning and then crashes after it. It may be because I am emitting the unreachable in the wrong place (I am using the param location) but if I was then the warning wouldn't be emitted, right? Perhaps I need the location of the basic block's entry point instead of the param. I could be wrong, but I'll investigate this further soon.

When I try emitting the IR then I don't get this crash.

[theblixguy]

@jrose-apple I have fixed the issue and now it works fine! 🎉

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 297a35d

[jrose-apple]

@jckarter should probably take one more look, but this looks good to me. May be worth commenting about why we can assume the unreachable code is due to the function having an uninhabited parameter, though.

@swift-ci Please test

[swift-ci]

Build failed
Swift Test Linux Platform
Git Sha - 4f39e3b

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 4f39e3b

[Azoy]

I know Jordan doesn't particularly like the word uninhabited in diagnostics, but this case seems uncommon so I'm okay with this.

[jrose-apple]

Heh, it slipped by me in looking at the code. That's true; it would probably be nicer to say "is an enum with no cases" or "contains an enum with no cases" as appropriate.

[theblixguy]

Hmm, I have noticed both "uninhabited" being used in diagnostics (ex: missing_never_call) as well as "enum with no cases" (ex: type_inferred_to_uninhabited_type), so I am not sure. "Uninhabited" seems succinct (but its type theory jargon) where as "enum with no cases" seems swifty (i.e. that's how you create an uninhabited type in Swift - but also seems like an implementation detail), but in either case, it's the same thing really. I can amend the diagnostic once again if required!

[jrose-apple]

Unrelated Foundation failure, I think…

@swift-ci Please test Linux

[theblixguy]

Unrelated Foundation failure, I think…

Yeah, all good now!

[jrose-apple]

I don't expect anything to come up here, but

@swift-ci Please test source compatibility

[theblixguy]

I don't expect anything to come up here, but

That took a long time haha, but all good here as well 👌

[theblixguy]

Is this good to merge now? @jrose-apple @jckarter

[jckarter]

Yeah, looks good. Thanks @theblixguy!

[mdiep]

Awesome! 🎉