Re: Copy/assignment of sc_object

Philipp,

Okay, I buy that. Since I agree that this is a loophole in the current standard and no-one else has expressed an opinion, I will implement the change as you suggest.

John A

-----"Philipp A. Hartmann" <philipp.hartmann@offis.de> wrote: -----
To: john.aynsley@doulos.com
From: "Philipp A. Hartmann" <philipp.hartmann@offis.de>
Date: 11/04/2010 12:23PM
Cc: david.long@doulos.com, P1666 Technical WG <systemc-p1666-technical@eda.org>
Subject: Re: Copy/assignment of sc_object

John,

with my proposed semantics of the copy ctor and assignment operator all
invariants of an sc_object would actually hold.

  Therefore, a user-defined class derived from sc_object does not need
to implement these operations explicitly to guarantee the correct behaviour:

struct my_object : sc_object
{
   sc_my_object()
     : sc_object("my_object") {}

   // compiler-generated copy ctor/assignment work as they should
};

  Making the operations private and adding rules how to achieve
copyable/assignable sc_object derivates does not seem to be less effort.
 If we make these operations private, I would prefer to add a rule
somewhere, that the semantics of working around this restriction
manually is undefined.

Greetings from Oldenburg,
  Philipp

On 03/11/10 23:13, john.aynsley@doulos.com wrote:
> Philipp, Dave,
>
> My mistake about the copy ctors. I misread the LRM.
>
> I agree about the Rule-of-Three.
>
> Regarding the constraints on the hierarchical relationships and names, I'm not sure which way we should go. Having raw sc_objects in an application (rather than sc_module, sc_port etc) has always been an anomaly in my mind; an open-ended mechanism that applications might have a use for. If an application chooses to construct and then copy raw sc_objects, why should it not have control over where those objects sit in the object hierarchy? On the other hand, the LRM has explicit rules defining the parent and the name for each sc_object, however it may have been constructed (4.1.4 and 6.16.1). I guess the most consistent approach would be to keep the existing rules, i.e.
>
> * If an sc_object is constructed from a module constructor, the module is its parent
> * If an sc_object is constructed from a process, the process instance is its parent
>
> I suppose we could add an explicit statement that these rules must be honored by any user-defined class derived from sc_object (though that would be true anyway by inference). Similarly we could spell out that the hierarchical name shall be unique (though again that would be true anyway).
>
> But is there really a use case either for assignment or for copy construction? What do they buy you, given the possible confusion over object name and object parent. Why not just disable them both? That would seem the simpler solution, rather than fussing around with invariants that must be honored by user-defined classes.
>
> John A
>
>
> -----"Philipp A. Hartmann" <philipp.hartmann@offis.de> wrote: -----
> To: john.aynsley@doulos.com
> From: "Philipp A. Hartmann" <philipp.hartmann@offis.de>
> Date: 11/02/2010 05:59PM
> Cc: david.long@doulos.com, P1666 Technical WG <systemc-p1666-technical@eda.org>
> Subject: Re: Copy/assignment of sc_object
>
> John,
>
> On 03/11/10 01:19, john.aynsley@doulos.com wrote:
>> We have:
>>
>> 1666-2005 LRM:
>> sc_object has protected copy ctor but no operator=
>> sc_module has neither copy ctor nor operator=
>
> Sorry, but tn my copy of the latest of your drafts, I don't find a
> definition of a copy ctor in sc_object. The only constructors
> (protected indeed) of sc_object are:
>
> sc_object();
> sc_object(const char*);
>
>> OSCI sim:
>> sc_object has protected copy ctor but no operator=
>
> Same here: neither is defined.
>
>> sc_module has private copy ctor but no operator=
>
> Confirmed.
>
>> Firstly, please excuse my ignorance concerning C++. Do we
>> need to add the operator= to the LRM and OSCI sim, or
>> are the semantics of assignment somehow inferred from the
>> copy ctor, or am I missing something?
>
> I agree, that digging such information from the C++ standard is quite
> tedious. But the implicitly declared copy-assignment operator is
> generated regardless of the existence of a copy constructor. Moreover,
> it is not defined in terms of a copy constructor. Instead:
>
> 12.8(13) [class.copy] says:
> The implicitly-defined copy assignment operator for class X performs
> memberwise assignment of its subobjects.
>
> This distinction is (somewhat) important, since you can have objects,
> that are Assignable, bot not Copyable. Consequently, there is the
> so-called Rule-of-Three:
>
> If your class defines a destructor, copy-constructor, or
> assignment operator, you should probably define all three.
>
>> Secondly, you are proposing we make both copy ctor and operator=
>> private ("disabled") in both sc_object and sc_module, right?
>
> I would propose to make both private for sc_module, yes.
> For sc_object, Dave had the valid point that there are certain
> use-cases, where copying an sc_object can help.
>
> My proposal would be to add a protected copy ctor and assignment
> operator with something like the following description:
>
> sc_object( const sc_object & src );
>
> The copy constructor shall create a new sc_object instance, as
> if it would have been created by 'sc_object( src.basename() )'.
> In particular, this means the location of the newly created instance
> is a child object of the current module (during elaboration) or
> process (during simulation). Attributes or child objects of the
> source object shall not be copied.
>
> sc_object& operator=( const sc_object& );
>
> The assignment operator shall not modify the hierarchical name
> or the location of the destination object in the object hierarchy.
> Attributes and child objects of the destination object shall not
> be changed. A reference to *this shall be returned.
>
> NOTE -- The copy constructor and assignment operator are provided as
> convenience functions for copyable/assignable user-defined
> defined objects derived from sc_object. Without those, such
> objects would need to explicitly define copy/assignment
> operations regardless whether the implicitly generated ones
> would suffice otherwise.
>
> Opinions? (The wording can of course be improved)
>
>> I would be fine with that. Does everyone agree?
>>
>> If so, could you please provide a somewhat more detailed
>> description of the user-defined class to include as an example?
>
> If needed, I can of course provide a more detailed example.
> But I would prefer to have the pre-defined copy. :-)
>
>>
>> -----"Philipp A. Hartmann" <philipp.hartmann@offis.de> wrote: -----
>> To: david.long@doulos.com
>> From: "Philipp A. Hartmann" <philipp.hartmann@offis.de>
>> Date: 10/26/2010 10:46AM
>> Cc: John Aynsley <john.aynsley@doulos.com>, owner-systemc-p1666-technical@eda.org, SystemC P1666 Technical <systemc-p1666-technical@eda.org>
>> Subject: Re: Copy/assignment of sc_object
>>
>> David,
>>
>> Thanks for your feedback.
>>
>> I can see the use-cases for copying objects derived from sc_object.
>> But from the standard's perspective, we need to define the correct
>> semantics not only for the parent object and the (non-existent)
>> children, but also for the name and the attributes of an sc_object.
>>
>> The crash in my example occurs due to a double delete of the name
>> member in the destructor, since the compiler-generated copy is only a
>> shallow one.
>>
>> If we make these operations private, a user-defined class can still
>> define their own copy-semantics as follows (by ignoring/replacing the
>> base operations):
>>
>> struct some_object : sc_object
>> {
>> // ...
>>
>> some_object( const some_object & that )
>> // use a "similar" name in the correct hierarchical position
>> : sc_object( that.basename() )
>> { /* ... */ }
>>
>> some_object& operator=( const some_object& that )
>> {
>> // do NOT call (as the default operator would)
>> // sc_object::operator=( that );
>> // ...
>> return *this;
>> }
>> };
>>
>> OTOH, if we define these operations explicitly in the standard, we need
>> to ensure more or less the same semantics as in the manual
>> implementation above. Specifically, the following sc_object invariants
>> should be guaranteed:
>>
>> - all object names in the hierarchy are still unique
>>
>> - the hierarchical name correctly reflects the position in the
>> hierarchy, and this position depends on the context of (copy)
>> creation, not on the position of the source object
>>
>> - attributes shall NOT be copied (or replaced during assignment)
>> This is debatable, though. But if we want to deep-copy
>> attributes, sc_attribute_base needs a clone() function.
>>
>> - upon assignment, name and parent object of the destination object
>> shall NOT change
>>
>> Since we are quite late in the standardisation process, it may be
>> easier to make these operations private, adding a note with the manual
>> example as outlined above.
>>
>> The attached patch implements protected copy/assignment with the
>> properties discussed above in the latest OSCI simulator and avoids the
>> crash in my initial example.
>>
>> Greetings from Oldenburg,
>> Philipp
>>
>> On 26/10/10 17:54, david.long@doulos.com wrote:
>>>
>>> I think that it should be allowed to copy a user-defined class derived
>>> from sc_object. For example, consider if an application wanted to create
>>> something like the ovm_object/uvm_object class that is used as a base
>>> class for transactions and sequence items in those SV methodologies (I am
>>> not talking about TLM2 generic_payload type transactions here) - why
>>> should these not be copied? The links to their parent object would still
>>> be valid.
>>>
>>> The 2005 LRM already states that "only module objects and process objects
>>> can have children" (5.15.1) so I don't think this introduces any
>>> anomalies.
>>>
>>> As you point out, what is currently missing are explicit definitions for
>>> the sc_object copy constructor and assignment operator, but I think they
>>> should be protected, not private. The virtual destructor (which is what
>>> causes the crash in your example) is already declared as protected in the
>>> LRM.
>>>
>>>
>>> Best Regards,
>>>
>>> Dave Long
>>> Doulos
>>>
>>>
>>>
>>>
>>> From:
>>> "Philipp A. Hartmann" <philipp.hartmann@offis.de>
>>> To:
>>> John Aynsley <john.aynsley@doulos.com>
>>> Cc:
>>> SystemC P1666 Technical <systemc-p1666-technical@eda.org>
>>> Date:
>>> 26/10/2010 14:50
>>> Subject:
>>> Copy/assignment of sc_object
>>> Sent by:
>>> owner-systemc-p1666-technical@eda.org
>>>
>>>
>>>
>>> John,
>>>
>>> since users can derive their own classes from sc_object, but such
>>> objects still have an explicit identity within the object hierarchy,
>>> sc_object's copy constructor/copy-assignment operator should probably be
>>> disabled, right?
>>>
>>> Is this an oversight in 1666-2005? I think, in sc_object's class
>>> definition (6.15.3), we should add
>>>
>>> private:
>>> // disabled
>>> sc_object( const sc_object& );
>>> sc_object& operator=( const sc_object& );
>>>
>>> The attached test program compiles fine and crashes (after pressing
>>> <enter>) with OSCI's PoC implementation.
>>>
>>> NB: sc_module is missing explicitly disabled copy/assignment as well.
>>> But these are correctly disabled in the OSCI simulator.
>>>
>>> I think, if we would want to support copying and assigning user-defined
>>> sc_objects, we would need to define the semantics wrt. the object
>>> hierarchy explicitly.
>>>
>>> What do you think?
>>>
>>> Greetings from Oldenburg,
>>> Philipp
>>>
>>
>>
>
>

-- 
Philipp A. Hartmann
Hardware/Software Design Methodology Group
OFFIS Institute for Information Technology
R&D Division Transportation ú FuE-Bereich Verkehr
Escherweg 2 ú 26121 Oldenburg ú Germany
Phone/Fax: +49-441-9722-420/282 ú PGP: 0x9161A5C0 ú http://www.offis.de/
 
-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.