I was surprised to find that the following snippet compiles:
fun main(args: Array<String>) {
test {
println("hey!")
return
println("Strangely this compiles even though it's right after the return statement")
}
println("The compiler should complain about this too")
}
inline fun test(body: () -> Unit) {
body()
}
Having a statement after a return fails to compile in Java so I would have expected the Kotlin compiler to abort as well. Additionally, no warnings were presented (but this really should be a compilation error).
This seems pretty severe since it will cause surprises (especially if it’s not as obvious as performing another statement right after returning).
I’m using IntelliJ 2017.2.2 with Kotlin plugin 1.1.60
Most compilers issue a warning about unreachable code rather than stop compiling. There are valid reasons for having unreachable code, such as disabling a section of code that may be re-enabled. It’s hardly a critical error.
I can see why such a decision would be made for languages that have text based macros however this doesn’t apply to Java or Kotlin. This enables Java & Kotlin to provide additional robustness guarantees that wouldn’t normally be possible with text based macros.
I can also see the value in allowing you to control if some code should be executed inside a condition which is controlled by a boolean constant. Something like this could be used to enable a testing mode so that additional tests are performed when its enabled.
However, any statement directly after a return in Kotlin is likely a human error so having the compiler complain about it would reduce potential errors. If you want to disable a section of code, you could easily comment it out.
Given that many of the Kotlin users are coming from Java, and that Java prevents compiling such code, I would suggest that fixing this would make Kotlin a better language.
I personally like it – when I’m debugging a method and I want to manually return a value early (instead of calculating the value as usual) I can just plop return foo in the middle somewhere and it still works without having to comment out a bunch of code. Plus I doubt they can change it now – some code surely is relying on this behavior, though perhaps they could change it in a new major version. e.g. 1.2.
As for the code itself, well, here’s what I get, which seems painfully obvious to me:
So this is closer to the structure of my original code. This occurred because I went back and added an early return inside the lambda without reconsidering the follow-on code (and IntelliJ didn’t present any warning so I didn’t think much about it).
Realizing the problem afterwards, I went back to see how broken the system was by adding another statement immediately after the return so I didn’t look out for warnings again. So you are correct that a warning is presented if you have a statement right after the return within the same block (which is fairly easy to spot) but unfortunately my scenario isn’t handled correctly.
Although I would prefer an error, the compiler should at the very least issue a warning about the unreachable code after the lambda.
Hm…you’re right, technically the code following the return is unreachable. Perhaps it’s too complicated for the compiler to do that level of unreachable code analysis – for instance, if the body() call was wrapped in any sort of conditional, it’d be impossible to infer if the second println() was unreachable or not. Might be interesting if the compiler did some of these checks after inlining the method call, but I have no idea if that’s practical.
You do not get unreachable code warning on println("This doesn't get executed") because the compiler cannot infer that it’s unreachable. When analyzing the body of main function, the compiler doesn’t know anything about how the test function is implemented and whether it calls body function at all.
There’s a plan to provide the way to declare that test will definitely call body once invoked, so that the code after such test invocation as in your example will be marked as unreachable.
Since the compiler warns about unreachable code for a statement after a return within the same block, it seems like this problem can be fixed if the unreachable-code detection logic is performed after the inlining step (instead of before).
The compiler already complains if I try to add a return statement in the lambda when the “test” function is not declared as inline so it seems like it’s already half-way there.
What are you basing your “half-way there” estimate on, becuase in general it’s impossible to write a compiler that can detect all unreachable code. Even if you don’t write for the general case, the compiler would likely be very slow if it tried so find most cases of unreachable code, and would take many person-years of effort to write.
The estimated difficulty is based on the fact that the compiler already has the smarts to detect this kind of problem if the detection phase was run after the inlining phase (because the inlining phase would transform the code into a format which matches a pattern that the compiler already detects).
So fixing this problem might be as simple as just changing the order of the steps so that the inlining step happens before the unreachable-code detection step.
Given that inlining operates on the bytecode level, it’s not as simple as it seems.
Even if it was possible to reverse these steps, the cost of doing inlining and then analyzing the resulting code would be prohibitively high, especially in IDE where you need to analyze the code many times during editing.
