[Sema] ReturnStmt must be ImplicitConversionExpr aware

[davezarzycki]

Found while doing an unrelated experiment.

[davezarzycki]

@swift-ci please smoke test

[slavapestov]

Test case?

[davezarzycki]

Hi @slavapestov – I can't think of a way to generate an implicit conversion outside of my private research branch (which created an implicit conversion expression as a side effect of other changes). Can you think of a way of testing this? Otherwise this just seems like good defensive coding.

[slavapestov]

I'm not sure I agree that this is defensive coding. I tend to delete code whose behavior cannot be observed via the test suite.

[davezarzycki]

In theory, I agree with you – but in practice I don't think that is a realistic position. For example, if a user (by accident or on purpose) creates an implicit conversion expression as a part of the return statement, then they can bypass the escape check. That's sucks.

For whatever it may be worth, I've run into this bug before, albeit elsewhere in the code. The problem is that Sema is often unprepared for implicit conversion expressions due to raw isa or dyn_cast usage. I try to fix specific diagnostics as I trip over them, but honestly, a more holistic solution is needed. (Perhaps something like the Type type hierarchy? Where one must use getAs instead of dyn_cast?)

[slavapestov]

Ok, I guess it can’t hurt. Without tears though, we can’t promise not to break the behavior used by your private branch later.

[slavapestov]

Without tests* but tears works too :)

[davezarzycki]

I'm not afraid of breakage. That's the cost of non-merged code (private or public). I was trying to think of a way to generate a FunctionConversionExpr, but I ran out of time. Thanks for merging this.