I propose that assert and assertionFailure should be no-ops (with branch
hints) in unchecked builds as they are in normal release builds rather
than resulting in undefined behaviour in the failure condition.
I would like to kick off a discussion of this. I found the proposal
template useful for setting out my thoughts clearly to trigger the
discussion but I'm not trying jump the discussion phase. I'm open to
radical rewrites or to not proceed if there is opposition. Also if my
understanding of the current code is wrong please let me know.
[Current Situation]
The documented behaviour of assert and assertionFailure in "disable
safety checks" builds (still documented as -Ounchecked) is that the
compiler "may assume that it would evaluate to true" or in the
assertionFailure case never be called.
This documented behaviour would allow the compiler to completely
eliminate tests and branches before or after the assertion and take the
operation deep into undefined behaviour.
https://github.com/apple/swift/blob/cf8baedee2b09c9dd2d9c5519bf61629d1f6ebc8/stdlib/public/core/Assert.swift
(latest commit to this file at time of writing)
[NOTE - Actual current behaviour]
It appears from the code as if the assumption is not currently applied
on the assert method although it is on the assertionFailure case by
means of the _conditionallyUnreachable() call. assert seems to be a
no-op in both normal release and disable safety checks build modes.
It also appears that precondition does not apply the permitted
assumption when in _isFastAssertConfiguration mode.
There also appears to be code in assert to hint the compiler that the
assert will likely be true. This was something that I was planning to
suggest and means that code containing asserts can be faster than the
same code without the assert in release mode.
[Proposed Change]
Change the documentation for assert and assertionFailure so that
behaviour in unchecked mode is the same as in normal release - no
evaluation and no effect.
Change the behaviour of assertionFailure to match the updated documentation.
[Motivation]
1) Expected behaviour based on other languages is for assert to have no
effect in release. (If current behaviour is highly desired another
function name should be used). Having potential dangerous behaviour from
a function that is familiar across languages and is regarded as a safety
feature is undesirable.
2) Adding asserts to code should not make the code more dangerous
whatever the build. Assuming the truth of the assert may lead to runtime
safety checks being skipped and undefined behaviour when a no-op would
be a safe behaviour.
3) "For highly robust code assert and then handle the error anyway"
[Code Complete 2nd Edition section 8.2] While the designed expectation
from the Swift team is to use them for checks against internal
programming errors within a module in a number of cases I use assertions
and assertionFailure calls while processing input data. The code without
the assertion would simply fail to read the input gracefully but as the
developer I want to know immediately if there are realistic cases that
have not been handled so use assertions to flag it to myself that a case
may need better handling. As such there are assertions in my code that
are in error cases that I expect not to occur. I do not want the runtime
safety checks being optimised out. [Being aware of this issue I use
custom assertions but am therefore missing out on the branch hinting of
the stdlib version and others may want similar behaviour and may not
fully read the documentation].
[Impact on existing code]
Loss of performance in cases where assertionFailure is used. Loss of
potential performance improvement option in assert usage. In most cases
this performance loss will be small but there is potential where the
assumption of the value allows large code blocks (to evaluate the
condition) to be eliminated for significant effects but I suspect that
these cases are rare and preconditionFailure could be used instead to
get the performance in unchecked builds (at the cost of release
performance) or an additional method could be added.
[Alternatives]
Renaming assert so that the behaviour is not assumed by users familiar
with other languages. Function name suggestion:
assume/assumeUnreachable
This could either behaviour as the current assert/assertionFailure
documentation or possibly allow the assumption to be made in normal
release mode not just the unchecked. It should be a fatal error in debug
builds if assumption is incorrect.
[Note about precondition]
precondition/preconditionFailure also have undefined behaviour in
unchecked mode but this is not a problem for me for a couple of reasons:
1) I don't have the same prior understanding about what they do.
2) It is clear from release build behaviour that hitting the condition
is always an error.
_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution
