[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-multimedia
Subject:    Re: 'default ' keyword working.
From:       Stefan Westerfeld <stefan () space ! twc ! de>
Date:       2000-03-21 16:44:39
[Download RAW message or body]

   Hi!

On Tue, Mar 21, 2000 at 02:10:12PM +0000, Nicolas Brodu wrote:
> Stefan Westerfeld wrote:
> 
> > > The connection asserts that the number of defaults match for the input/output of
> > > the connected objects, when the ports aren't specified explicitely. Another
> > > possibility would be to do as many connections as possible between the source
> > > and destination objects. This would partially solve the inheritance issue above,
> > > but then one must be more careful when connecting 2 objects. What do you prefer?
> > 
> > Is there something that speaks against this behaviour:
> > - if you don't mention the keyword default in your object declaration, the
> >   default ports are left as they were in the parent object
> 
> Except in the case the parents have no default port and the object has only one
> => it becomes the default. But yes, I'll code up this fine behaviour.
> 
> > - if you do, the default ports are set to what you say, so you don't honor
> >   parents default ports any longer
> 
> That would be fine, but you cannot then add a default to the parent ones. Hmm,
> we could honor only the default ports specified in the idl as you suggest here,
> but in the case a string does not match one of the ports of the object, then
> look if it comes from a parent default. This would be even cleaner since it
> allows to do exactly what you want in the inherited objects, and keep the
> possibility to reuse parent ports!
> Is there a simple way to get the parent interfaces in mcopidl (they could come
> from an included idl)? If not, I'll do the test at run-time (in the generated
> code), after all, default ports are used only during connect so the small
> overload won't matter that much.

Sure you can access the definitions of the parent interfaces. The
InterfaceDef's from include files are read just like the InterfaceDef's
for the interface you are generating code for.

> > I think that might be a clear behaviour. Of course, if you inherit from two
> > objects which both have default streams, then you loose - but hey, we can't
> > do magic ;)
> 
> With the behaviour above, you won't loose ;-)

Okay, that sounds good.


> > > On another topic, I didn't have time to investigate but it seems that the
> > > smartwrapper code have a minor reference counting bug somewhere for remote
> > > connection. More precisely, artscat ends up after a few seconds when the
> > > reference clean finds a not so 'unused' object and frees it.
> > 
> > I thought a bit about reference stuff (without reading the code too deeply),
> > and I see a few issues that smartwrappers maybe don't handle correct yet.
> > 
> > ==> 1. Returncodes vs. Parameters:
> > 
> > Object_base *doSomething(Object_base *foo)
> 
> Well, I modified the Returncodes & Parameters to use smartwrappers, but since
> there is no smartwrapper for Object I kept Object_base* instead. That's the
> reason of the inconsistency I think.
I wanted to say that the same is valid for any other thing. Also for

Foo_base *doSomething(Foo_base *foo)

> What changes would be necessary to generate
> a smartwrapper for Object? (if needs be, we could call it Object_wrap to keep
> the Object class as it is now).

Yes, we need that sooner or later. Also all smartwrappers without parent
interfaces should inherit this "Object" smartwrapper then, so that you don't
need to redeclare node() and toString() and such.

However, I would like to wait with such a global change until we know what
position smartwrappers will have in the whole concept. If you want to try
it at home, you can uncomment the Object interface in core.idl, mcopidl
core.idl and merge the result into object.cc/object.h and recomment the
interface. However don't put it into the CVS right now.

> > ==> 2. Null references
> > 
> > I think there are issues with null references. The current smartwrappers
> > often do
> > 
> >   if(!nullreference) assign(...);
> > 
> > however, I would be amazed if I do:
> > 
> > Synth_PLAY p;
> > p = remoteObject.doSomething();
> > if(p.isNull()) cerr << "no object" << endl;
> > 
> > and doSomething *really* returns a null reference. What would happen is that
> > upon calling p.isNull(), auto-creation kicks in, and you don't get the error
> > msg, but a fresh locally created object. Thus, you need to do
> >
> >   if(nullreference) assign(0);
> >   else assign(reference->_copy());
> > 
> > in all those cases.
> 
> The only solution I see now involve spliting internally isNull() into nullity
> and error conditions. But then, there is overload because the error flag is
> positionned after each method call. Now that I think about it, it would make
> code more robust. But is it what we want (is the robustness worth the small
> extra overload)? 

The solution I see is to do correct null assignments. If you do assign(0) in
the 2nd line inside the smartwrapper correctly, then the assign will deny
auto creation (as every assign sets _autocreate to false), and p.isNull()
in line three will succeed. You just shouldn't simply ignore if somebody
assigns a null reference to a smartwrapper!

> > ==> 3. "Fixed" autocreation?
> > 
> > void calc(Synth_FREQUENCY param)
> > {
> >     // 2. a copy is made, but left unassigned
> >     setFloatValue(&param,440.0);  // 3. the copy gets autocreated here
> >     // 4. the copy gets destroyed here
> > }
> > 
> > main()
> > {
> >     Synth_FREQUENCY f; // 1. f is unassigned (but ready to autocreate) here
> >     calc(f);           // 5. f is still unassigned here
> >     f.start();         // 6. f is now created, however without 440 Hz setting
> > }
> > 
> > I think this is a new bug in the fixed auto creation. You need to do the
> > creation upon copy. (As the program should work exactly the same as if f
> > would get created in 1. already).
> 
> That's no bug! It's standard C++ issue. void calc(int a) would not modify 'a' in
> the caller as well. In this example, void calc(Synth_FREQUENCY& param) is
> correct, because you want 'calc' to do something on the main 'f'.

From the behaviour, I think of smartwrappers mostly as

typedef Synth_FREQUENCY_base* Synth_FREQUENCY;

void calc(Synth_FREQUENCY param)
{
	param->_node()->setFloatValue("freq",440.0);
	/* calling param methods (like setValue) will modify the same param
	   object as the caller has */

	param = 0;
	/* modifying param itself won't do anything to the caller */
}

Smartwrappers *are* references to implementations.

If I write

  SimpleSoundServer server(Reference("global:Arts_SimpleSoundServer"));

then this means:

  "Make server a reference to the already existing object
   Arts_SimpleSoundServer".

That means: All smart wrappers are from their behaviour the same as
references/pointers.

The following behaviour (which the CVS smartwrappers expose currently) is
for that reason correct!! (Plain C++ impls would behave differently):

   Hello h(4);
   Hello j = h;

   j.sum(2);   // add 2
   j.total();  // prints 6
   h.total();  // prints 6, too!

Through the assignment, they "refer" to the same implementation. Working
that way required, because otherwise they could never ever access remote
objects at all.

The current CVSs (also correct) behaviour is also the following:

void calc(Synth_FREQUENCY param)
{
    setFloatValue(&param,440.0);  // 3. param refers to f here
}

main()
{
    Synth_FREQUENCY f; // 1. f is unassigned (but ready to autocreate) here
	setFloatValue(&f,220.0);  // 2. f is created here
    calc(f);
    f.start();         // 4. f *has* 440 Hz setting here!!
}

It's just auto-creation that gets in the way of the first example I mentioned,
and makes it "work" differently.

Mapping idl

interface X {
  void x(Y y);
}

to

void X::x(Y& y)

is not an option, as if the implementation of y contains a changing of the
implementation y refers to remote and local usage will behave differently.
For instance:

void X::x(Y& y)
{
   y = (Y_base *)0;  // bad style, but somebody could have the idea to do that
}

   X xr(Reference(some remote object));
   Y a(4);
   xr.x(a);
   /*
    * a.isNull() is false here (as all parameters will only be transferred
    * from client to server, and not back again)
    */

   X xl(Reference(some local object));
   Y b(4);
   xl.x(b);
   /* b.isNull() is true here */

Smartwrappers (and arts _var constructs) should have the same semantics as
passing an Y_base*, not an Y_base*&. You can change the value of object it
is pointing to (through invoking methods on that object which change the
value), but not the pointer itself. I'd really like to see them usable as

void X::x(const Y& y)

as strings are also done, as this would significantly reduce the overhead
in passing them as parameters, but autocreation gets in the way here with
the non-const operations. Perhaps dropping auto-creation and have some
kind of explicit creation again is the better way. What do you think?

   Cu... Stefan
-- 
  -* Stefan Westerfeld, stefan@space.twc.de (PGP!), Hamburg/Germany
     KDE Developer, project infos at http://space.twc.de/~stefan/kde *-

[prev in list] [next in list] [prev in thread] [next in thread]