http://d.puremagic.com/issues/show_bug.cgi?id=302

           Summary: in/out contract inheritance yet to be implemented
           Product: D
           Version: 0.165
          Platform: PC
               URL: http://www.digitalmars.com/d/dbc.html
        OS/Version: Windows
            Status: NEW
          Keywords: wrong-code
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: bugzilla@digitalmars.com
        ReportedBy: smjg@iname.com


A long-standing unimplemented feature is inheritance of in/out contracts.  From the spec:

"If a function in a derived class overrides a function in its super class, then only one of the in contracts of the function and its base functions must be satisified. Overriding functions then becomes a process of loosening the in contracts.

A function without an in contract means that any values of the function parameters are allowed. This implies that if any function in an inheritance heirarchy [sic] has no in contract, then in contracts on functions overriding it have no useful effect.

Conversely, all of the out contracts needs [sic] to be satisified, so overriding functions becomes a processes of tightening the out contracts."

It's about time we finally got this implemented.  A strategy was proposed at digitalmars.D:31595, "Implementing contract inheritance".

Existing DStress testcases: http://dstress.kuehne.cn/run/i/in_out_body_10_A.d http://dstress.kuehne.cn/norun/i/in_out_body_10_B.d http://dstress.kuehne.cn/norun/i/in_out_body_10_C.d http://dstress.kuehne.cn/run/i/in_out_body_11_*.d


-- 

d-bugmail@puremagic.com wrote:
> 
> "If a function in a derived class overrides a function in its super class, then
> only one of the in contracts of the function and its base functions must be
> satisified. Overriding functions then becomes a process of loosening the in contracts.
> 
> A function without an in contract means that any values of the function
> parameters are allowed. This implies that if any function in an inheritance
> heirarchy [sic] has no in contract, then in contracts on functions overriding
> it have no useful effect.
> 

Does this makes sense? If in-contracts should be less restrictive than their parents (and they should), why allow in-contracts that are not less restrictive? Isn't this behavior more adequate:
«In a derived function, *all* in-contracts must be satisfied.
If the first in-contract fails, then it is a programmer error on regards to using the function.
If the first in-contract passes, but any parent in-contract fails, then there is an error in the function specification itself, since the in-contracts are not less restrictive.»

Consider this example:

-----
class Foo {
  void cfunc(int a)
  in {
    assert(a != 42);
  } body {
    writefln("Foo.cfunc: != 42 ", a);
  }

}

class FooBar : Foo {
  override void cfunc(int a)
  in {
    assert(a != 666);
  } body {
    writefln("FooBar.cfunc: != 666 ", a);
  }
}
-----

The in-contract of derived cfunc is not conceptually valid, but DMD allows it. It would be nice to detect that at compile-time but that is likely not feasible in the general-case, so perhaps we could opt for the aforementioned behavior, that would detect this at runtime?


-- 
Bruno Medeiros - MSc in CS/E student
http://www.prowiki.org/wiki4d/wiki.cgi?BrunoMedeiros#D

Bruno Medeiros wrote:
<snip>
> Does this makes sense? If in-contracts should be less restrictive than their parents (and they should), why allow in-contracts that are not less restrictive? Isn't this behavior more adequate:
> «In a derived function, *all* in-contracts must be satisfied.

Basic OO principle: whatever is doable on an object of a base class
should be doable on an object of any class derived from it.

> If the first in-contract fails, then it is a programmer error on regards to using the function.
> If the first in-contract passes, but any parent in-contract fails, then there is an error in the function specification itself, since the in-contracts are not less restrictive.»

Which is the "first" in-contract?
- the one of the hierarchy level where the function is introduced in the
first place?
- the one in the class of which the object reference is declared to be?
- the one in the actual class of the object?

And what does "parent" mean - base or derived?  I would have thought
base, but you seem to be using it to mean derived.

Perhaps:

"If the in-contract in the class of the object reference fails, then it is an error from the caller's point of view.  If this in-contract passes, but the in-contract in any derived class fails, then there is an error in the function specification itself, since the in-contract of the derived class imposes new restrictions."

However, this leads to code duplication if the derived class wants to leave the base class's in-contract unchanged.  (Which is bound to be the case if the derived class function unconditionally calls the base class function on the same arguments and then does more.)  By the current spec, one can achieve this by simply using

    in {
        assert (false);
    }

in the derived class.  Which indeed is cryptic, but can you think of a better idea?  Moreover, what if you're deriving from a library class, and the library author improves the function in the base class to work in more cases.  Should you be able to use this improved functionality straight out of the box?

> Consider this example:
> 
> -----
> class Foo {
>   void cfunc(int a)
>   in {
>     assert(a != 42);
>   } body {
>     writefln("Foo.cfunc: != 42 ", a);
>   }
> 
> }

So on any object of class Foo, cfunc can be called with any argument
that is not 42.

> class FooBar : Foo {
>   override void cfunc(int a)
>   in {
>     assert(a != 666);
>   } body {
>     writefln("FooBar.cfunc: != 666 ", a);
>   }
> }
> -----
> 
> The in-contract of derived cfunc is not conceptually valid, but DMD allows it. It would be nice to detect that at compile-time but that is likely not feasible in the general-case, so perhaps we could opt for the aforementioned behavior, that would detect this at runtime?

But how?  When a function in a derived class is called, should it call all the in-contracts leading down to it, to make sure that all in-contracts from a certain level down to it pass and all in-contracts above it fail?

A further matter of debate is whether, given

    Qwert yuiop = new Asdfg;
    yuiop.hjkl();

the in-contract from Qwert (type of the object reference) or Asdfg (actual class of the object) should be used.  Of course, the natural way to implement it depends on whether the in-contract is enforced on the caller side or the callee side.

Since the programmer has used a Qwert reference, then it's more a matter of using Qwert correctly than of using Asdfg correctly.  Another advantage of enforcing in-contracts on the caller side is that one build of a library can sensibly be used for both development and release builds of applications, and the application programmer would still have the checking that he/she/it is using the library correctly.

Walter once criticised the idea of doing it this way claiming that it leads to "code bloat", but I'm not convinced it's serious enough (considering the amount of code bloat in development builds anyway) to outweigh the benefits.

Stewart.

-- 
-----BEGIN GEEK CODE BLOCK-----
Version: 3.1
GCS/M d- s:-@ C++@ a->--- UB@ P+ L E@ W++@ N+++ o K-@ w++@ O? M V? PS-
PE- Y? PGP- t- 5? X? R b DI? D G e++++ h-- r-- !y
------END GEEK CODE BLOCK------

My e-mail is valid but not my primary mailbox.  Please keep replies on
the 'group where everyone may benefit.

Stewart Gordon wrote:
> Bruno Medeiros wrote:
> <snip>
>> Does this makes sense? If in-contracts should be less restrictive than their parents (and they should), why allow in-contracts that are not less restrictive? Isn't this behavior more adequate:
>> «In a derived function, *all* in-contracts must be satisfied.
> 
> Basic OO principle: whatever is doable on an object of a base class
> should be doable on an object of any class derived from it.
> 
>> If the first in-contract fails, then it is a programmer error on regards to using the function.
>> If the first in-contract passes, but any parent in-contract fails, then there is an error in the function specification itself, since the in-contracts are not less restrictive.»
> 

Oops, I erred writing that description above. -_-'  See below.

> Which is the "first" in-contract?
> - the one of the hierarchy level where the function is introduced in the
> first place?
> - the one in the class of which the object reference is declared to be?
> - the one in the actual class of the object?
> 

"first" meant the most-derived, that is, the first in-contract starting from the method being called to the base methods along the overriding hierarchy(lineage).

> And what does "parent" mean - base or derived?  I would have thought
> base, but you seem to be using it to mean derived.
> 

I meant base, but you are right, the description above is wrong. Among other things I inverted the behaviors, so the terms "first" and "parent" didn't seem right. Should be:

«In a derived method, all in-contracts are checked. Then:

If all in-contracts fail, then it is an error regarding the usage of method (caller error).

If some in-contract fails, but any subsequent parent in-contracts pass, then there is an error in the method specification itself, since the in-contracts are not less restrictive. [A runtime error is thrown too?]

If none of the above, which is when one or more of the most-derived method in-contracts pass, and all (zero or more) subsequent parent in-contracts fail, the preconditions are accepted.»

>> Consider this example:
>>
>> -----
>> class Foo {
>>   void cfunc(int a)
>>   in {
>>     assert(a != 42);
>>   } body {
>>     writefln("Foo.cfunc: != 42 ", a);
>>   }
>>
>> }
>
> So on any object of class Foo, cfunc can be called with any argument
> that is not 42.
>
>> class FooBar : Foo {
>>   override void cfunc(int a)
>>   in {
>>     assert(a != 666);
>>   } body {
>>     writefln("FooBar.cfunc: != 666 ", a);
>>   }
>> }
>> -----
>>
>> The in-contract of derived cfunc is not conceptually valid, but DMD
>> allows it. It would be nice to detect that at compile-time but that is
>> likely not feasible in the general-case, so perhaps we could opt for
>> the aforementioned behavior, that would detect this at runtime?
>
> But how?  When a function in a derived class is called, should it call
> all the in-contracts leading down to it, to make sure that all
> in-contracts from a certain level down to it pass and all in-contracts
> above it fail?
>

It should work the like the behavior stated above, which is roughly like what you said.

> Perhaps:
> 
> "If the in-contract in the class of the object reference fails, then it is an error from the caller's point of view.  If this in-contract passes, but the in-contract in any derived class fails, then there is an error in the function specification itself, since the in-contract of the derived class imposes new restrictions."
> 

The checks have to be made from the called method upwards along the lineage to the parent methods, and not downwards from the called method, as that is not correct nor even possible, as you don't know the derived classes of any given class.

> However, this leads to code duplication if the derived class wants to leave the base class's in-contract unchanged.  (Which is bound to be the case if the derived class function unconditionally calls the base class function on the same arguments and then does more.)  By the current spec, one can achieve this by simply using
> 
>     in {
>         assert (false);
>     }
> 
> in the derived class.  Which indeed is cryptic, but can you think of a better idea?  Moreover, what if you're deriving from a library class, and the library author improves the function in the base class to work in more cases.  Should you be able to use this improved functionality straight out of the box?
> 

What I was thinking in regards to this, is, if the in-contract of a method is not specified, then it is the same as the parent's one, that is, it is inherited. "not specified" means not writing the 'in' clause.
And when one wants to specify an empty pre-condition, use "in { }".


-- 
Bruno Medeiros - MSc in CS/E student
http://www.prowiki.org/wiki4d/wiki.cgi?BrunoMedeiros#D

http://d.puremagic.com/issues/show_bug.cgi?id=302


smjg@iname.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|normal                      |major




-- 

http://d.puremagic.com/issues/show_bug.cgi?id=302


clugdbug@yahoo.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|wrong-code                  |




------- Comment #1 from clugdbug@yahoo.com.au  2009-04-22 04:41 -------
This isn't wrong-code. (I agree that it's important, but it's not bad code generation -- marking this sort of thing as wrong-code makes the wrong-code keyword useless).


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=302





------- Comment #2 from smjg@iname.com  2009-04-22 06:46 -------
"it's not bad code generation" - how do you work that out?


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=302





------- Comment #3 from clugdbug@yahoo.com.au  2009-04-22 07:34 -------
(In reply to comment #2)
> "it's not bad code generation" - how do you work that out?

Bad code generation generally means the front-end is sending incorrect data to the backend; or else there's a bug in the backend. (Also for the case of CTFE, it can also include bugs in the interpreter; in that case, the frontend is the backend).

In this case, the bug is really in the spec: the spec states a vague intention about how contracts should work, but there's never been any attempt whatsoever to implement it in the compiler. At the very least, the spec should say that it's not implemented, and I suspect that it should be completely removed from the D1 spec, unless Walter actually intends to implement it in D1.

I notice that these unimplemented things (array operations was the other obvious one, but I suspect there are others) all date from the very early days of D. In modern times, Walter's mostly changed the spec only when he's actually implemented something.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=302





------- Comment #4 from fvbommel@wxs.nl  2009-04-22 07:41 -------
The spec specifies what the code should do, but the code being generated does
something else. How is that not the very definition of wrong-code?
(This probably falls in the "the front-end is sending incorrect data to
the backend" category)

Never having been implemented doesn't change any of this, IMHO.


-- 

http://d.puremagic.com/issues/show_bug.cgi?id=302


clugdbug@yahoo.com.au changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |spec




------- Comment #5 from clugdbug@yahoo.com.au  2009-04-22 08:25 -------
(In reply to comment #4)
> The spec specifies what the code should do, but the code being generated does
> something else. How is that not the very definition of wrong-code?
> (This probably falls in the "the front-end is sending incorrect data to
> the backend" category)
> 
> Never having been implemented doesn't change any of this, IMHO.

I think the problem is in the spec in this case. When it's not implemented, it should either marked as not yet implemented in DMD, or it shouldn't be in the spec at all yet. _Every_ other case of wrong-code in bugzilla is different. The nature of the issue is completely different, so I really think it's unhelpful to use the same keyword for both.


--