Remove unsafe precondition check

[kyouko-taiga]

This precondition check is triggering undefined behavior in some cases where the arguments of the call do not match the signature of the callee.

LLVM performs the precondition check on its own anyway when assertions are enabled. So I think the precondition check can be removed entirely.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.31%. Comparing base (118a78e) to head (399ac9d).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #137      +/-   ##
==========================================
+ Coverage   85.17%   85.31%   +0.13%     
==========================================
  Files          57       57              
  Lines        1410     1403       -7     
==========================================
- Hits         1201     1197       -4     
+ Misses        209      206       -3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[tothambrus11]

Precondition violations indicate bugs. Are these incorrectly stated preconditions?

How can this ever trigger UB? How can we be sure LLVM's assertions don't trigger UB?

[kyouko-taiga]

These preconditions are at least insufficient. If the types of the arguments do not match the types of the parameters, then the call will go through and violate LLVM's preconditions. It seems also that nothing is done if FunctionType.UnsafeReference(calleeType) returns nil, meaning that an incorrect callee type will go unnoticed.

Because LLVM can trigger UB on a whim, I am not sure it is worth adding incomplete precondition checks before all the calls we perform. I'd rather treat the entire API of Swifty LLVM as unsafe rather than giving a false sense of security. The reason is that it took me longer to figure out what was going wrong. The debugger never reached LLVM and your precondition check made me discard the obvious. I had to scan through all jumps in the assembly to get on the right track.

After I removed your precondition, assertions fired consistently in LLVM. I suspect (but have no confirmation) that the optimizer is doing something funny when your check precedes the call to LLVM. If my theory holds, then we should reconsider the benefit of all the extra checks that we have. There are burdensome to maintain, add run-time costs (casting is not free), and we can't make SwiftyLLVM safe anyway.

[tothambrus11]

The issue here is that non-copyable objects, including self: Module have non-lexical lifetimes, and inout parameters may be deinitialized before the end of the function if it is deemed not to be used after some point. There is no use of self within the branch that realizes the failure, so the destruction of self is hoisted to the beginning.

A proper solution would be to borrow self e.g. using a function call with a borrowing parameter that would extend the lifetime. I don't know if this is something we could improve we could improve with language design. It looks sensible, but it's also a massive footgun for unsafe code.

[tothambrus11]

I'll close this, as the UB is removed by #139. We can remove the checks later if it turns out to be a noticeable performance overhead.