www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - You're Doing In-Conditions Wrong

reply FeepingCreature <feepingcreature gmail.com> writes:
Reposting my bug report 
https://issues.dlang.org/show_bug.cgi?id=20628 here to generate 
some discussion.

tl;dr: D is handling in-contracts in a way that causes bad 
outcomes both with and without debug mode, causing bizarre logic 
errors and even accepting invalid syntax.

---

Background:

An in-condition is part of D's implementation of Design by 
Contract. As such, their use on class methods acts as an 
extension of Liskov's Substitution Principle, which 
(generalizedly) states that class methods that override other 
methods may take more kinds of inputs, and return less kinds of 
outputs, than their parent method.

That is, a method that does not take 'null' values may be 
overridden by one that takes 'null' values, by the logic of "at 
least all parent's inputs are supported."

Reversedly, a method that may return 'null' values may be 
overridden by one that does not return 'null' values, by the 
inverse logic of "at most all parent's outputs are possible."

In other words, methods may, when inherited, *loosen* their input 
but *tighten* their output.

D lets us formalize this:

class A {
   Object method(Object obj)
   in (obj !is null)
   // no restriction on output
   ;
}

class B : A {
   override Object method(Object obj)
   // loosen restriction on input: null is allowed
   // tighten restriction on output: null is not allowed
   out (result; result !is null)
   ;
}

For inconditions, D implements this behavior as follows:

1. Check the superclass in-contract.
2. If the superclass contract throws an exception:
2.1. Then retry with the class's own in-contract.

This seems sensible. However, in practice it causes several 
problems.

Let's consider two cases: debug modes, where we want to look for 
and find logic errors, and non-debug mode, where we just want to 
run correctly.

Within debug mode, D should enforce that in contracts loosen the 
conditions. As such, it should always execute both superclass and 
subclass contract and Error if superclass-in passes but 
subclass-in does not.

In other words:

try {
   superMethod.runInCondition();
} catch (Exception) {
   // if this throws, all is well - the input is just not accepted.
   method.runInCondition();
   // if it didn't throw, all is well - the overridden in contract 
loosened the condition
   return;
}
// super method accepted this input - confirm that our logic holds
try {
   method.runInCondition();
} catch (Exception) {
   throw new LogicError("in contract was tightened in subclass - 
this is illegitimate");
}

But what to do outside debug mode? In my opinion, the reasonable 
thing to do is to only run the child contract.

Why? Well, consider the case that the parent contract is more 
tight than the child contract. In that case, the parent may throw 
an informative exception, but we don't care because we ignore it 
anyway; here, the parent's contract provides no information.

However, consider the case that the parent contract is more loose 
than the child contract. In that case, the child contract has 
*more* information than the parent. In the debug case, we want a 
LogicError in this scenario. However, the second-best thing is 
the exception that the child provides. Again, the parent contract 
has nothing to add, and we should just run the child contract.

Further benefits: this will also fix weirdnesses such as

interface I { void foo(); }
class C : I { void foo() in(this.is.never.compiled) { } }

or

interface I { void foo() in(true); }
class C : I { void foo() in(false) { } }

which would then be a compiletime error or runtime error, 
respectively.

Programming languages should do reasonable things. No reasonable 
programming language, when faced with this code,

void foo(int i)
in (i > 5)
{
   assert(i > 5, "this cannot happen");
}

should ever fail with "this cannot happen".
Jul 13 2020
next sibling parent reply Panke <tobias pankrath.net> writes:
On Monday, 13 July 2020 at 13:55:56 UTC, FeepingCreature wrote:

 But what to do outside debug mode? In my opinion, the 
 reasonable thing to do is to only run the child contract.



I don't want any contract checking outside of debug mode. You 
need to check your real inputs independent of the compilation 
mode and the in-contract is not the right place to do it.

On the other hand, having additional checks that are only 'on' in 
debug mode is useful.
Jul 13 2020
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Monday, 13 July 2020 at 14:29:45 UTC, Panke wrote:

 I don't want any contract checking outside of debug mode.


contracts are actually independent compiler switches you can turn 
on and off separate from everything else. This is just talking 
about two layers of checks, don't read too much into the term 
"debug mode".
Jul 13 2020
parent FeepingCreature <feepingcreature gmail.com> writes:
On Monday, 13 July 2020 at 14:33:42 UTC, Adam D. Ruppe wrote:

 On Monday, 13 July 2020 at 14:29:45 UTC, Panke wrote:

 I don't want any contract checking outside of debug mode.


 contracts are actually independent compiler switches you can 
 turn on and off separate from everything else. This is just 
 talking about two layers of checks, don't read too much into 
 the term "debug mode".


The idea here would be that the "expensive" two-step check would 
be limited to `-debug`, though I guess you could just always do 
it that way.

My ordering of preference is "check both in debug, check child 
without debug" > "check both (with LogicError/LiskovError) 
always" > "only check child always" > "current behavior".
Jul 13 2020
prev sibling next sibling parent reply burt <invalid_email_address cab.abc> writes:
On Monday, 13 July 2020 at 13:55:56 UTC, FeepingCreature wrote:

 [...]

 Let's consider two cases: debug modes, where we want to look 
 for and find logic errors, and non-debug mode, where we just 
 want to run correctly.

 Within debug mode, D should enforce that in contracts loosen 
 the conditions. As such, it should always execute both 
 superclass and subclass contract and Error if superclass-in 
 passes but subclass-in does not.


I don't believe this is actually the case; it should not throw an 
Error if superclass-in passes and subclass-in does not. Consider 
the following case:

```
class A {
     /// Postconditions: `x` must equal `2`.
     void method(int x) in (x == 2) {}
}
class B : A {
     void method(int x) in (x == 3) {}
}

auto b = new B;
b.method(3); // valid
b.method(2); // valid, since `B` is-an `A` and `A.method` 
guarantees that inputs that pass x == 2 are valid.

A a = b;
a.method(2); // valid, even though this passes superclass-in but 
not subclass-in
a.method(3); // dangerous! `A.method` does not guarantee that x = 
3 is allowed!
```

This is perfectly valid code, since the signature of `A.method` 
guarantees that it may take any inputs that are `x == 2`. The 
relationship is "super-in || sub-in" (an OR-relationship). [0]


 In other words:

 try {
   superMethod.runInCondition();
 } catch (Exception) {
   // if this throws, all is well - the input is just not 
 accepted.
   method.runInCondition();
   // if it didn't throw, all is well - the overridden in 
 contract loosened the condition
   return;
 }
 // super method accepted this input - confirm that our logic 
 holds
 try {
   method.runInCondition();
 } catch (Exception) {
   throw new LogicError("in contract was tightened in subclass - 
 this is illegitimate");
 }
 But what to do outside debug mode? In my opinion, the 
 reasonable thing to do is to only run the child contract.


This is not possible: input that passes the `in`-contract of the 
superclass should also be guaranteed to be valid input, if the 
child contract does not pass. After all, inputs that are valid 
for the superclass's implementation should also be valid for the 
subclass's implementation.


 Why? Well, consider the case that the parent contract is more 
 tight than the child contract. In that case, the parent may 
 throw an informative exception, but we don't care because we 
 ignore it anyway; here, the parent's contract provides no 
 information.

 However, consider the case that the parent contract is more 
 loose than the child contract. In that case, the child contract 
 has *more* information than the parent. In the debug case, we 
 want a LogicError in this scenario. However, the second-best 
 thing is the exception that the child provides. Again, the 
 parent contract has nothing to add, and we should just run the 
 child contract.

 Further benefits: this will also fix weirdnesses such as

 interface I { void foo(); }
 class C : I { void foo() in(this.is.never.compiled) { } }


This is valid, but should probably emit an error, since `I.foo` 
has no in-contracts and will always pass, so `C.foo`'s condition 
will never have to be checked (like you said).


 interface I { void foo() in(true); }
 class C : I { void foo() in(false) { } }
 which would then be a compiletime error or runtime error, 
 respectively.


This is exactly the same case as above: `I.foo` will always pass 
and as a result, `C.foo` does not have to be checked.


 Programming languages should do reasonable things. No 
 reasonable programming language, when faced with this code,

 void foo(int i)
 in (i > 5)
 {
   assert(i > 5, "this cannot happen");
 }

 should ever fail with "this cannot happen".


In fact, when in-contracts are disabled, `foo(5)` may decide to 
fail with "this cannot happen", crash, corrupt memory or fail 
with "ERROR", since continuing execution while the in-contract 
has failed is UB anyway!

-burt

[0] https://dlang.org/spec/contracts.html#in_out_inheritance
Jul 14 2020
parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Tuesday, 14 July 2020 at 10:19:51 UTC, burt wrote:

 On Monday, 13 July 2020 at 13:55:56 UTC, FeepingCreature wrote:

 [...]

 Let's consider two cases: debug modes, where we want to look 
 for and find logic errors, and non-debug mode, where we just 
 want to run correctly.

 Within debug mode, D should enforce that in contracts loosen 
 the conditions. As such, it should always execute both 
 superclass and subclass contract and Error if superclass-in 
 passes but subclass-in does not.


 I don't believe this is actually the case; it should not throw 
 an Error if superclass-in passes and subclass-in does not. 
 Consider the following case:

 [snip]


I think the disagreement here is whether an incondition should 
mean "a condition for the method" or "a condition that is added 
to the implicit disjunction of the parent inconditions."

I think the way that D works currently is bad. I'm raising a 
design criticism here, not a bug - I know the current behavior is 
per spec. But I mean, if you see

```
class B : A {
     void method(int x) in (x == 3) {}
}
```

You don't expect x to be 2. In fact, the vastly more plausible 
way to arrive at this code is that the parent used to check `x == 
3` but was changed to check `x == 2`. The disjunctive approach 
gives up the chance to discover this bug, for no benefit. Why no 
benefit? To be frank, because this kind of example essentially 
never comes up in practice.

Something like 95% of inconditions in our codebase at least, are 
some variant of "not null". How do you relax this incondition? By 
not writing anything, in both proposals. You certainly don't 
write `in (obj is null)`.

When do you want to add an additional disjunctive check that is 
totally unrelated to the parent's check? Even if you're say, 
expanding an enum, the expanded check will simply be "is the 
value in the expanded enum," not "is the value one of the two new 
enum values that I added."

I think "restate the parent's condition plus your new values" is 
already what people do anyways. Might as well take advantage.
Jul 14 2020
next sibling parent FeepingCreature <feepingcreature gmail.com> writes:
On Tuesday, 14 July 2020 at 12:05:23 UTC, FeepingCreature wrote:

 Something like 95% of inconditions in our codebase at least, 
 are some variant of "not null". How do you relax this 
 incondition? By not writing anything, in both proposals. You 
 certainly don't write `in (obj is null)`.


Addendum: Compare the type system.

If you have

```
class A
{
   void foo(A a) { }
}
class B
{
   override void foo(B b) { }
}
```

You wouldn't expect b to be typed "B or A". No, you'd expect to 
get an error. Parameter types are contravariant, not extensive; 
inconditions should follow the same logic.
Jul 14 2020
prev sibling parent burt <invalid_email_address cab.abc> writes:
On Tuesday, 14 July 2020 at 12:05:23 UTC, FeepingCreature wrote:

 [...]

 I think the disagreement here is whether an incondition should 
 mean "a condition for the method" or "a condition that is added 
 to the implicit disjunction of the parent inconditions."

 I think the way that D works currently is bad. I'm raising a 
 design criticism here, not a bug - I know the current behavior 
 is per spec. But I mean, if you see

 ```
 class B : A {
     void method(int x) in (x == 3) {}
 }
 ```

 You don't expect x to be 2. In fact, the vastly more plausible 
 way to arrive at this code is that the parent used to check `x 
 == 3` but was changed to check `x == 2`. The disjunctive 
 approach gives up the chance to discover this bug, for no 
 benefit. Why no benefit? To be frank, because this kind of 
 example essentially never comes up in practice.


So should the writer of a subclass HAVE to write out the 
preconditions of the parent? Because that could be impossible 
sometimes, if the precondition includes a call to a private 
function or something. Or perhaps some way to explicitly call the 
superclass's in contract.


 Something like 95% of inconditions in our codebase at least, 
 are some variant of "not null". How do you relax this 
 incondition? By not writing anything, in both proposals. You 
 certainly don't write `in (obj is null)`.


You could also write `in (true)` in the current state of affairs.


 When do you want to add an additional disjunctive check that is 
 totally unrelated to the parent's check? Even if you're say, 
 expanding an enum, the expanded check will simply be "is the 
 value in the expanded enum," not "is the value one of the two 
 new enum values that I added."

 I think "restate the parent's condition plus your new values" 
 is already what people do anyways. Might as well take advantage.


Is that sufficient for all cases? What if the superclass's 
precondition is `input1 == input2`, and the subclass's 
precondition also want to allow 
`input1.equalsCaseInsensitive(input2)` or something, how would 
you write that out?

-burt
Jul 14 2020
prev sibling parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 7/13/20 9:55 AM, FeepingCreature wrote:

 Reposting my bug report https://issues.dlang.org/show_bug.cgi?id=20628 
 here to generate some discussion.
 
 tl;dr: D is handling in-contracts in a way that causes bad outcomes both 
 with and without debug mode, causing bizarre logic errors and even 
 accepting invalid syntax.


I think you mean, invalid code, not syntax. Invalid syntax will not pass 
the parser.

But I somewhat disagree. Yes, you can write bad contracts, but that is 
not on the compiler, and can't really be checked by the compiler. The 
compiler enforces the rule by ignoring what the derived class does if 
the parent class passes. It doesn't enforce the logic of your contract 
fits the rule.

However, the biggest problem I have with contract inheritance is that no 
contract means "ignore base contract". Why is this a problem? Because 
people are lazy and don't do things they don't have to.

If you do nothing, what are the chances that
   a) you know about the contract, and actively decided in(true) is the 
correct contract for your subtype, and also knew that not providing a 
contract was the equivalent of in(true) so just didn't write it.
   b) you forgot/didn't notice it.

IMO, an unstated contract should not alter the parent contract. Unstated 
code means "I didn't care" or "leave it the same". It's not an active 
decision to alter the contract drastically to "accept everything".

Possibly this means changing a base contract could cause problems with 
existing code. I'm OK with that, it is you changing the requirements for 
your users (the authors of the derived code).

However, it is tedious that one has to repeat all the super's contract 
if your additive contract is unrelated.

One possibility is to consider a way to say "everything super said and ..."

maybe like:

void foo(int i) in(super.in) in(i > 5) {}

-Steve
Jul 14 2020
next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Tuesday, 14 July 2020 at 13:37:58 UTC, Steven Schveighoffer 
wrote:

 If you do nothing, what are the chances that
   a) you know about the contract, and actively decided in(true) 
 is the correct contract for your subtype, and also knew that 
 not providing a contract was the equivalent of in(true) so just 
 didn't write it.


It is actually `in(false)` to inherit the parent's contract. 
in(true) means you accept anything and everything. kinda nuts lol

http://dpldocs.info/this-week-in-d/Blog.Posted_2019_12_02.html
Jul 14 2020
parent Steven Schveighoffer <schveiguy gmail.com> writes:
On 7/14/20 9:49 AM, Adam D. Ruppe wrote:

 On Tuesday, 14 July 2020 at 13:37:58 UTC, Steven Schveighoffer wrote:

 If you do nothing, what are the chances that
   a) you know about the contract, and actively decided in(true) is the 
 correct contract for your subtype, and also knew that not providing a 
 contract was the equivalent of in(true) so just didn't write it.


 
 It is actually `in(false)` to inherit the parent's contract. in(true) 
 means you accept anything and everything. kinda nuts lol
 
 http://dpldocs.info/this-week-in-d/Blog.Posted_2019_12_02.html


Not providing a contract is in(true). My point is, what do you think the 
chances that not providing a contract when the parent class does means 
that you want to accept all inputs, or that you didn't care about 
contracts at all?

You should have to explicitly say in(true) in the derived class if 
that's what you intended.

-Steve
Jul 14 2020
prev sibling parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Tuesday, 14 July 2020 at 13:37:58 UTC, Steven Schveighoffer 
wrote:

 I think you mean, invalid code, not syntax. Invalid syntax will 
 not pass the parser.


Ah, yes, right.


 But I somewhat disagree. Yes, you can write bad contracts, but 
 that is not on the compiler, and can't really be checked by the 
 compiler. The compiler enforces the rule by ignoring what the 
 derived class does if the parent class passes. It doesn't 
 enforce the logic of your contract fits the rule.


It can be checked by the compiler just fine at runtime. IMO, this 
is what unittests are for.


 However, it is tedious that one has to repeat all the super's 
 contract if your additive contract is unrelated.

 One possibility is to consider a way to say "everything super 
 said and ..."

 maybe like:

 void foo(int i) in(super.in) in(i > 5) {}

 -Steve


That would also be nice, but it would be nice regardless of my 
proposal.
Jul 14 2020
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 7/14/20 11:37 AM, FeepingCreature wrote:

 On Tuesday, 14 July 2020 at 13:37:58 UTC, Steven Schveighoffer wrote:

 But I somewhat disagree. Yes, you can write bad contracts, but that is 
 not on the compiler, and can't really be checked by the compiler. The 
 compiler enforces the rule by ignoring what the derived class does if 
 the parent class passes. It doesn't enforce the logic of your contract 
 fits the rule.


 
 It can be checked by the compiler just fine at runtime. IMO, this is 
 what unittests are for.


But the derived contracts are intended to be used only if the base 
contract fails. It's not entirely inappropriate or unheard of to write 
such a contract with that in mind.

What you are asking for is to require the derived contracts to implement 
the base contract's conditions that allow the same inputs.

Let's look at an example similar to what you wrote:

class A
{
     void foo(int i) in (i == 3) {} // accept only 3
}

class B : A
{
     override void foo(int i) in (i == 2) {} // accept 3 OR 2.
}

If the intention of B is to accept 3 or 2, then it is written correctly. 
Your proposal wants to say that B should only be valid if written like:

class B : A
{
     override void foo(int i) in (i == 2 || i == 3) {}
}

This would be like getting rid of else if:

if(i == 2) { }
/*else*/ if(i != 2 && i == 3) {}


 

 However, it is tedious that one has to repeat all the super's contract 
 if your additive contract is unrelated.

 One possibility is to consider a way to say "everything super said and 
 ..."

 maybe like:

 void foo(int i) in(super.in) in(i > 5) {}


 
 That would also be nice, but it would be nice regardless of my proposal.


Actually, I take this back. I don't think we need this (obviously, if we 
get to this contract, super.in has failed, there's no reason to test it 
again). What is needed is testing part of the parent contract, which can 
only be done via encapsulation of the test into a function that can be 
called.

I still think the biggest problem with contracts is the no-contract 
handling for derived types. It's just too easy to erase the base 
contract by accident.

-Steve
Jul 14 2020
parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Tuesday, 14 July 2020 at 16:09:59 UTC, Steven Schveighoffer 
wrote:

 On 7/14/20 11:37 AM, FeepingCreature wrote:

 On Tuesday, 14 July 2020 at 13:37:58 UTC, Steven Schveighoffer 
 wrote:

 But I somewhat disagree. Yes, you can write bad contracts, 
 but that is not on the compiler, and can't really be checked 
 by the compiler. The compiler enforces the rule by ignoring 
 what the derived class does if the parent class passes. It 
 doesn't enforce the logic of your contract fits the rule.


 
 It can be checked by the compiler just fine at runtime. IMO, 
 this is what unittests are for.


 But the derived contracts are intended to be used only if the 
 base contract fails. It's not entirely inappropriate or unheard 
 of to write such a contract with that in mind.


Right, I agree that this is what it's intended for, I just think 
that's a bad intent. It's not *entirely* inappropriate or unheard 
to write such a contract, but I do think it's *almost* entirely 
inappropriate and unheard. Do you have any practical examples, 
not contrived i==3 cases?
Jul 14 2020
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 7/15/20 12:17 AM, FeepingCreature wrote:

 On Tuesday, 14 July 2020 at 16:09:59 UTC, Steven Schveighoffer wrote:

 On 7/14/20 11:37 AM, FeepingCreature wrote:

 On Tuesday, 14 July 2020 at 13:37:58 UTC, Steven Schveighoffer wrote:

 But I somewhat disagree. Yes, you can write bad contracts, but that 
 is not on the compiler, and can't really be checked by the compiler. 
 The compiler enforces the rule by ignoring what the derived class 
 does if the parent class passes. It doesn't enforce the logic of 
 your contract fits the rule.


 It can be checked by the compiler just fine at runtime. IMO, this is 
 what unittests are for.


 But the derived contracts are intended to be used only if the base 
 contract fails. It's not entirely inappropriate or unheard of to write 
 such a contract with that in mind.


 
 Right, I agree that this is what it's intended for, I just think that's 
 a bad intent.


Why is not requiring you to restate the base code contract a bad intent? 
Note that the restating would have to be done carefully, as you don't 
want to fail the derived contract in cases where you have loosened the 
requirements.


 It's not *entirely* inappropriate or unheard to write such 
 a contract, but I do think it's *almost* entirely inappropriate and 
 unheard. Do you have any practical examples, not contrived i==3 cases?


My knowledge is only theoretical -- I never use contracts, just in-code 
asserts. And I rarely use classes anyway.

However, any time I have used contracts, I have gotten frustrated with 
how they disappear because code authors (mostly me) forget to include 
them on the derived types. They might get used more if they weren't so 
easy to get rid of.

I get what you are saying -- having a contract right in front of you 
that is seemingly violated is confusing and unintuitive, even if it is 
correct. I don't know a good answer, but I don't like the idea of 
requiring less DRY code.

-Steve
Jul 15 2020
parent reply Timon Gehr <timon.gehr gmx.ch> writes:
On 15.07.20 14:00, Steven Schveighoffer wrote:

 On 7/15/20 12:17 AM, FeepingCreature wrote:

 On Tuesday, 14 July 2020 at 16:09:59 UTC, Steven Schveighoffer wrote:

 On 7/14/20 11:37 AM, FeepingCreature wrote:

 On Tuesday, 14 July 2020 at 13:37:58 UTC, Steven Schveighoffer wrote:

 But I somewhat disagree. Yes, you can write bad contracts, but that 
 is not on the compiler, and can't really be checked by the 
 compiler. The compiler enforces the rule by ignoring what the 
 derived class does if the parent class passes. It doesn't enforce 
 the logic of your contract fits the rule.


 It can be checked by the compiler just fine at runtime. IMO, this is 
 what unittests are for.


 But the derived contracts are intended to be used only if the base 
 contract fails. It's not entirely inappropriate or unheard of to 
 write such a contract with that in mind.


 Right, I agree that this is what it's intended for, I just think 
 that's a bad intent.


 
 Why is not requiring you to restate the base code contract a bad intent? 
 Note that the restating would have to be done carefully, as you don't 
 want to fail the derived contract in cases where you have loosened the 
 requirements.
 

 It's not *entirely* inappropriate or unheard to write such a contract, 
 but I do think it's *almost* entirely inappropriate and unheard. Do 
 you have any practical examples, not contrived i==3 cases?


 
 My knowledge is only theoretical -- I never use contracts, just in-code 
 asserts. And I rarely use classes anyway.
 
 However, any time I have used contracts, I have gotten frustrated with 
 how they disappear because code authors (mostly me) forget to include 
 them on the derived types. They might get used more if they weren't so 
 easy to get rid of.
 ...


Yes, this is broken.


 I get what you are saying -- having a contract right in front of you 
 that is seemingly violated is confusing and unintuitive, even if it is 
 correct. I don't know a good answer, but I don't like the idea of 
 requiring less DRY code.
 
 -Steve


Eiffel uses different syntax for require clauses on redefined features. 
"require else". In D, this would amount to something like:

override void foo()else in(i==3){ ... }
Jul 15 2020
parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Wednesday, 15 July 2020 at 14:31:30 UTC, Timon Gehr wrote:

 Eiffel uses different syntax for require clauses on redefined 
 features. "require else". In D, this would amount to something 
 like:

 override void foo()else in(i==3){ ... }


This seems like a good approach.

On Wednesday, 15 July 2020 at 12:00:49 UTC, Steven Schveighoffer 
wrote:

 Why is not requiring you to restate the base code contract a 
 bad intent?


Because it solves a problem that, as far as I can tell from our 
codebase which uses inconditions basically everywhere, doesn't 
exist.

I have, to my recollection, *never* wanted to loosen an 
incondition by adding a totally unrelated condition that can't be 
written as a different phrasing of the parent incondition. And we 
have, uh, *counts* ~4.3k inconditions. (Of which 3.7k are some 
variant of "is not null" tests.) Shouldn't D focus on the common 
case?
Jul 15 2020
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 7/16/20 2:26 AM, FeepingCreature wrote:

 On Wednesday, 15 July 2020 at 12:00:49 UTC, Steven Schveighoffer wrote:

 Why is not requiring you to restate the base code contract a bad intent?


 
 Because it solves a problem that, as far as I can tell from our codebase 
 which uses inconditions basically everywhere, doesn't exist.


If it doesn't do it this way, then the possibility exists that the 
derived class doesn't allow inputs that the base class does. Yes, you 
can test for it, but tests don't always prove the rule holds (if you 
don't test the right inputs). However, the current implementation 
*guarantees* that the rule holds, even if you don't properly handle it 
in your subcontract.

I agree that the way derived contracts are implemented, it's very 
difficult to see what the "actual" contract really is, because you have 
to scan through the entire object hierarchy to see what the full 
contract entails.

It's like having an if/else statement spread out over several modules.


 
 I have, to my recollection, *never* wanted to loosen an incondition by 
 adding a totally unrelated condition that can't be written as a 
 different phrasing of the parent incondition. And we have, uh, *counts* 
 ~4.3k inconditions. (Of which 3.7k are some variant of "is not null" 
 tests.) Shouldn't D focus on the common case?


I can see this being likely, but one project or organizational ecosystem 
is not proof that it doesn't exist. Every time I think "nobody would 
write code like this" and push to get rid of or change a feature, 
someone complains. There was e.g. significant resistance to getting rid 
of the comma operator.

I don't have a good answer. I don't know that implementing the contracts 
differently is going to achieve a better outcome. But I understand that 
the current situation is not ideal.

-Steve
Jul 16 2020
parent reply FeepingCreature <feepingcreature gmail.com> writes:
On Thursday, 16 July 2020 at 13:56:36 UTC, Steven Schveighoffer 
wrote:

 It's like having an if/else statement spread out over several 
 modules.


Worse: it's like having an if expression and body spread over 
several modules.


 I don't have a good answer. I don't know that implementing the 
 contracts differently is going to achieve a better outcome. But 
 I understand that the current situation is not ideal.


Well, the `else in()` idea from the other comment would work... 
problem is you can have multiple `in()` statements that are anded 
together, and then `else in` breaks down. So really, D went into 
a problematic direction many years ago.

Iunno. I feel changing the spec and adding an opt-in 
`InconditionLogicError`, with a very long deprecation period, 
would be a way we could start digging our way out of the hole 
again.
Jul 16 2020
parent FeepingCreature <feepingcreature gmail.com> writes:
I made a https://github.com/dlang/dmd/pull/11440 , just to test.

This does not assert on cases where child classes have no 
incondition at all, which should be an error - but at least the 
case "parent: in (i == 3)" with "child: in (i == 4)" does not 
actually seem to happen in practice, at least as far as buildkite 
can see.
Jul 21 2020