I gave a quick reply to an email later in the chain last night but I think 
these points are worth addressing. Apologies for the slow response, I wanted to 
ponder and consider the points rather than rush the response.

>> On Dec 31, 2015, at 8:27 PM, Chris Lattner <[email protected]> wrote:
>> 
>> On Dec 28, 2015, at 5:48 AM, Joseph Lord via swift-evolution 
>> <[email protected]> wrote:
>> 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.
> 
> Right.
> 
>> 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.
> 
> Only in cases where the assertion would have failed, right?  The point of 
> -Ounchecked is that - if your code was correct with the checks - that it will 
> still be correct.  Disabling overflow and array bounds checks is far more 
> dangerous than the assertion behavior you cite here.

Yes only when it would have failed but that may still make correct code 
incorrect where whole branches of runtime belt and braces code could be a 
eliminated. 

>> 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.
> 
> Right.
> 
>> [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.
> 
> Why? :
> 
>> 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.
> 
> This is the C model, but as you know, there is a whole field of custom 
> assertions libraries that people have developed.  I don’t think there is 
> anything like consensus on this topic.

C, Python, Erlang, Ocaml, Java (although it can enabled at runtime) and 
probably more. I recognise that in house assertion policies and systems may 
have different behaviours and agreed modes but I think affecting surrounding 
code (before and after the assertion) is surprising based on people experience 
with these other languages. I have not seen elsewhere any instances of 
compilers removing code outside of the branch itself which could be possible if 
the compiler is allowed to assume the truth of the assertion condition. 

if let names = jsonObject as? [String] where names.count > 0 {
  print(names.first)
} else {
  assertionFailure("Invalid JSON")
  return nil
}

Could be compiled to pretty much an unconditional print of an address in 
unchecked. The nil return could be unreachable, the if let reduced to an unsafe 
bitcast (maybe not given array bridging but possibly in other cases) and the 
.first converted to [0] given the assumed passing of the where clause.

>> 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.
> 
> This only affects code built with -Ounchecked, which is definitely not a safe 
> mode to build your code.  The intention of this mode is that you can use it 
> to get a performance boost, if you believe your code to be sufficiently 
> tested.  This mode, which isn’t the default in any way, intentionally takes 
> the guard rails off to get better performance.
> 
> If you don’t like that model, don’t use this mode

One fear is that libraries will be compiled in this way by people who aren't 
the original authors and that people won't realise the consequences.

>> 3) "For highly robust code assert and then handle the error anyway" [Code 
>> Complete 2nd Edition section 8.2]
> 
> Highly robust code shouldn’t build with -Ounchecked, so I don’t see how this 
> point is pertinent.

The quote was external validation of the concept of having asserts where the 
case is also handled. I would not limit the use to "highly robust code". 

However as I said in my other email if no one else is concerned I will just go 
away and use my custom assertions and miss out on the compiler hinting.

Joseph
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution

Reply via email to