www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - DIP 1006 - Preliminary Review Round 1

reply Mike Parker <aldacron gmail.com> writes:
DIP 1006 is titled "Providing more selective control over 
contracts".

https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

All review-related feedback on and discussion of the DIP should 
occur in this thread. The review period will end at 11:59 PM ET 
on April 26 (3:59 AM GMT), or when I make a post declaring it 
complete.

At the end of Round 1, if further review is deemed necessary, the 
DIP will be scheduled for another round. Otherwise, it will be 
queued for the formal review and evaluation by the language 
authors.

Thanks in advance to all who participate.

Destroy!
Apr 12
next sibling parent reply rikki cattermole <rikki cattermole.co.nz> writes:
On 12/04/2017 12:25 PM, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP should occur in
 this thread. The review period will end at 11:59 PM ET on April 26 (3:59
 AM GMT), or when I make a post declaring it complete.

 At the end of Round 1, if further review is deemed necessary, the DIP
 will be scheduled for another round. Otherwise, it will be queued for
 the formal review and evaluation by the language authors.

 Thanks in advance to all who participate.

 Destroy!
How exactly does this affect unittests? From what I can see, in none mode unittests won't have any asserts, which is clearly a problem.
Apr 12
next sibling parent reply Nicholas Wilson <iamthewilsonator hotmail.com> writes:
On Wednesday, 12 April 2017 at 11:32:37 UTC, rikki cattermole 
wrote:
 On 12/04/2017 12:25 PM, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over 
 contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP 
 should occur in
 this thread. The review period will end at 11:59 PM ET on 
 April 26 (3:59
 AM GMT), or when I make a post declaring it complete.

 At the end of Round 1, if further review is deemed necessary, 
 the DIP
 will be scheduled for another round. Otherwise, it will be 
 queued for
 the formal review and evaluation by the language authors.

 Thanks in advance to all who participate.

 Destroy!
How exactly does this affect unittests? From what I can see, in none mode unittests won't have any asserts, which is clearly a problem.
This is a -release style optimisation and would not be expected to be in dev/testing. Also unittest asserts call a different druntime function (IIRC) and therefore _should_ logically be under the control of -unittest and not affected by dip1006 OR be an invalid flag combination.
Apr 12
parent rikki cattermole <rikki cattermole.co.nz> writes:
On 12/04/2017 12:48 PM, Nicholas Wilson wrote:
 On Wednesday, 12 April 2017 at 11:32:37 UTC, rikki cattermole wrote:
 On 12/04/2017 12:25 PM, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP should occur in
 this thread. The review period will end at 11:59 PM ET on April 26 (3:59
 AM GMT), or when I make a post declaring it complete.

 At the end of Round 1, if further review is deemed necessary, the DIP
 will be scheduled for another round. Otherwise, it will be queued for
 the formal review and evaluation by the language authors.

 Thanks in advance to all who participate.

 Destroy!
How exactly does this affect unittests? From what I can see, in none mode unittests won't have any asserts, which is clearly a problem.
This is a -release style optimisation and would not be expected to be in dev/testing. Also unittest asserts call a different druntime function (IIRC) and therefore _should_ logically be under the control of -unittest and not affected by dip1006 OR be an invalid flag combination.
Please make it explicit that asserts are only in invariant and in/out blocks are affected. With a note about unittests, just to remove this possible incorrect implementation detail. I am not happy with the args passed to the switch, but I can't think of an alternative names, so I am happy with how the DIP is including the above statements.
Apr 12
prev sibling parent Mathias Lang <mathias.lang sociomantic.com> writes:
On Wednesday, 12 April 2017 at 11:32:37 UTC, rikki cattermole 
wrote:
 On 12/04/2017 12:25 PM, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over 
 contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP 
 should occur in
 this thread. The review period will end at 11:59 PM ET on 
 April 26 (3:59
 AM GMT), or when I make a post declaring it complete.

 At the end of Round 1, if further review is deemed necessary, 
 the DIP
 will be scheduled for another round. Otherwise, it will be 
 queued for
 the formal review and evaluation by the language authors.

 Thanks in advance to all who participate.

 Destroy!
How exactly does this affect unittests? From what I can see, in none mode unittests won't have any asserts, which is clearly a problem.
Good point. I would say that `-contracts=none` and `-unittest` should behave the same as `-release` and `-unittest`, and currently it only turns asserts on ( https://github.com/dlang/dmd/blob/ac3225a025b578d45ff39a40dda35006fb455a37/src/ddm /mars.d#L1100-L1109 ). I'll add a note about it in the DIP.
Apr 12
prev sibling next sibling parent Daniel Kozak <kozzi11 gmail.com> writes:
On Wednesday, 12 April 2017 at 11:25:09 UTC, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over 
 contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP should 
 occur in this thread. The review period will end at 11:59 PM ET 
 on April 26 (3:59 AM GMT), or when I make a post declaring it 
 complete.

 At the end of Round 1, if further review is deemed necessary, 
 the DIP will be scheduled for another round. Otherwise, it will 
 be queued for the formal review and evaluation by the language 
 authors.

 Thanks in advance to all who participate.

 Destroy!
Is this supposed to affect release build, debug build or both of them?
Apr 12
prev sibling next sibling parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 12.04.2017 13:25, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP should occur in
 this thread. The review period will end at 11:59 PM ET on April 26 (3:59
 AM GMT), or when I make a post declaring it complete.

 At the end of Round 1, if further review is deemed necessary, the DIP
 will be scheduled for another round. Otherwise, it will be queued for
 the formal review and evaluation by the language authors.

 Thanks in advance to all who participate.

 Destroy!
The DIP should probably specify in detail how this interacts with -release. I guess the difference between "-contracts=none -boundscheck=safeonly" and" -release" is that failing assertions are UB only with the latter? What happens if I pass both -contracts=none and -release?
Apr 12
parent reply Mathias Lang <mathias.lang sociomantic.com> writes:
On Wednesday, 12 April 2017 at 12:52:42 UTC, Timon Gehr wrote:
 On 12.04.2017 13:25, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over 
 contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP 
 should occur in
 this thread. The review period will end at 11:59 PM ET on 
 April 26 (3:59
 AM GMT), or when I make a post declaring it complete.

 At the end of Round 1, if further review is deemed necessary, 
 the DIP
 will be scheduled for another round. Otherwise, it will be 
 queued for
 the formal review and evaluation by the language authors.

 Thanks in advance to all who participate.

 Destroy!
The DIP should probably specify in detail how this interacts with -release. I guess the difference between "-contracts=none -boundscheck=safeonly" and" -release" is that failing assertions are UB only with the latter? What happens if I pass both -contracts=none and -release?
Addressing Daniel Kozak's question as well here: Providing `-release` on the command line has the following effect: - Disable invariants, - Disable in and out, - Disable assert, - Disable switch error [1] Which `-contract` just allows more control on. TL;DR: It affects all build. It's a subset of `-release`, so passing `-contracts=whatever` and `-release` has the same effect as passing `-release`. [1] Switch error: Compiler code here: https://github.com/dlang/dmd/blob/0158d32fcbbdda16b3f79e73f2f1f8c13afb9f6d/src/ddmd/statementsem.d#L2120-L2145 When a `switch has no `default,` in debug it'll insert a `default` which throws a `core.exception : SwitchError`, and in release the generated `default` will just be an `HLT` (same as `assert(0)`).
Apr 12
parent Timon Gehr <timon.gehr gmx.ch> writes:
On 12.04.2017 17:16, Mathias Lang wrote:

 Addressing Daniel Kozak's question as well here:

 Providing `-release` on the command line has the following effect:
 - Disable invariants,
 - Disable in and out,
 - Disable assert,
Actually, (at least) asserts are turned into compiler hints (i.e. potential UB).
 - Disable switch error [1]
 ...
I have missed that one. Note that it also implies -boundscheck=safeonly.
 Which `-contract` just allows more control on.

 TL;DR: It affects all build. It's a subset of `-release`, so passing
 `-contracts=whatever` and `-release` has the same effect as passing
 `-release`.
It's not a subset unless "disable" can mean "turn failures into undefined behaviour".
Apr 12
prev sibling next sibling parent reply Andrea Fontana <nospam example.com> writes:
On Wednesday, 12 April 2017 at 11:25:09 UTC, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over 
 contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP should 
 occur in this thread. The review period will end at 11:59 PM ET 
 on April 26 (3:59 AM GMT), or when I make a post declaring it 
 complete.

 At the end of Round 1, if further review is deemed necessary, 
 the DIP will be scheduled for another round. Otherwise, it will 
 be queued for the formal review and evaluation by the language 
 authors.

 Thanks in advance to all who participate.

 Destroy!
Why not --disable-contracts=invariant --disable-contracts=inout,invariants --disable-contracts=asserts,inout And so on?
Apr 12
parent reply Mathias Lang <mathias.lang sociomantic.com> writes:
On Wednesday, 12 April 2017 at 13:45:08 UTC, Andrea Fontana wrote:
 On Wednesday, 12 April 2017 at 11:25:09 UTC, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over 
 contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP 
 should occur in this thread. The review period will end at 
 11:59 PM ET on April 26 (3:59 AM GMT), or when I make a post 
 declaring it complete.

 At the end of Round 1, if further review is deemed necessary, 
 the DIP will be scheduled for another round. Otherwise, it 
 will be queued for the formal review and evaluation by the 
 language authors.

 Thanks in advance to all who participate.

 Destroy!
Why not --disable-contracts=invariant --disable-contracts=inout,invariants --disable-contracts=asserts,inout And so on?
It was a conscious decision to provide something simple to use, over something which allowed more control (good old KISS). If a use case for it develop in the future, the addition will be trivial.
Apr 12
parent Joseph Rushton Wakeling <joseph.wakeling webdrake.net> writes:
On Wednesday, 12 April 2017 at 15:37:14 UTC, Mathias Lang wrote:
 It was a conscious decision to provide something simple to use, 
 over something which allowed more control (good old KISS). If a 
 use case for it develop in the future, the addition will be 
 trivial.
Well, it's not simple to use if it doesn't fulfil your use-case. ;-) With that in mind, it would seem simpler overall to not make assumptions about use-cases, and just allow the user a free choice of what kinds of contract they disable: --disable-contracts=invariant,in,out,assert,all (Yes, I'm intentionally suggesting allowing `--disable-contracts=in`, `--disable-contracts=out`, and `--disable-contracts=in,out`.)
Apr 12
prev sibling next sibling parent reply Lewis <musicaljelly gmail.com> writes:
On Wednesday, 12 April 2017 at 11:25:09 UTC, Mike Parker wrote:
 DIP 1006 is titled "Providing more selective control over 
 contracts".

 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md

 All review-related feedback on and discussion of the DIP should 
 occur in this thread. The review period will end at 11:59 PM ET 
 on April 26 (3:59 AM GMT), or when I make a post declaring it 
 complete.

 At the end of Round 1, if further review is deemed necessary, 
 the DIP will be scheduled for another round. Otherwise, it will 
 be queued for the formal review and evaluation by the language 
 authors.

 Thanks in advance to all who participate.

 Destroy!
I have to ask the newbie question, just to make sure we're not missing anything obvious. Why can't we fix invariants so that they're pay-for-what-you-use? In other words, is there a way we can make sure _d_invariant is never called (or early-outs) for classes that don't use invariants?
Apr 12
parent Mathias Lang <mathias.lang sociomantic.com> writes:
On Wednesday, 12 April 2017 at 16:22:00 UTC, Lewis wrote:
 I have to ask the newbie question, just to make sure we're not 
 missing anything obvious. Why can't we fix invariants so that 
 they're pay-for-what-you-use? In other words, is there a way we 
 can make sure _d_invariant is never called (or early-outs) for 
 classes that don't use invariants?
There's no newbie question :) Sadly it is not possible. Consider the following hierarchy: ``` class Mother : Object { public int i; public void myFunction () {} } class Daughter1 : Mother { invariant () { assert(i != 0); } } class Daughter2 : Mother {} ``` The `Mother` class needs to insert invariant checks at the beginning and the end of `myFunction` to account for the possibility of a derived class defining invariant (here `Daughter1`). However those checks are superfluous if no `invariant` is defined, as it's the case with `Daughter2`. However the base class cannot know this in advance (because it might be in a library and already compiler, for example).
Apr 12
prev sibling parent "H. S. Teoh via Digitalmars-d" <digitalmars-d puremagic.com> writes:
On Wed, Apr 12, 2017 at 11:25:09AM +0000, Mike Parker via Digitalmars-d wrote:
 DIP 1006 is titled "Providing more selective control over contracts".
 
 https://github.com/dlang/DIPs/blob/master/DIPs/DIP1006.md
 
 All review-related feedback on and discussion of the DIP should occur
 in this thread. The review period will end at 11:59 PM ET on April 26
 (3:59 AM GMT), or when I make a post declaring it complete.
 
 At the end of Round 1, if further review is deemed necessary, the DIP
 will be scheduled for another round. Otherwise, it will be queued for
 the formal review and evaluation by the language authors.
 
 Thanks in advance to all who participate.
 
 Destroy!
Overall, I support the idea of this DIP. However, as others have mentioned, it needs to make it clear whether/how `-contracts=assert` here interacts with unittests. According to the discussion, apparently a different druntime function is used for asserts in unittests? If so, this needs to be clearly stated in the DIP. T -- People demand freedom of speech to make up for the freedom of thought which they avoid. -- Soren Aabye Kierkegaard (1813-1855)
Apr 12