NetNews Usenet Archive 1992 #30/NN_1992_30.iso/spool/comp/lang/cplus/18036

Text File | 1992-12-15 | 3.1 KB | 118 lines

Path: sparky!uunet!olivea!spool.mu.edu!sdd.hp.com!apollo.hp.com!netnews
From: [e-mail] (Stephen Vinoski)
Newsgroups: comp.lang.c++
Subject: Re: What is this CFRONT 3.0 warning message trying to tell me?
Message-ID: <[e-mail]>
Date: 16 Dec 92 02:43:50 GMT
References: <[e-mail]>
Sender: [e-mail] (Usenet News)
Organization: Hewlett-Packard Corporation, Chelmsford, MA
Lines: 105
Nntp-Posting-Host: srv.ch.apollo.hp.com

In article <[e-mail]> [e-mail] writes:
>I'm porting a large application from a CFRONT 2.1-based compiler (Sun) to
>a CFRONT 3.0-based compiler (Solbourne). The following warning message
>has me somewhat mystified:
>
>"main.cc", line 31: warning: temporary used for non-const Thing& argument; no changes will be propagated to actual argument (anachronism)
>
>I've boiled the offending code down to:
>
>#include <string.h>
>#include <malloc.h>
>#include <stdio.h>
>class Thing
>{
>   public:
>
>   char *str;
>   Thing(){ str = (char *)0; };
>   Thing(char *s){str=strdup(s);};
>   Thing &operator = ( Thing & rhs  );
>};
>
>Thing &
>Thing::operator = ( Thing & rhs )
>{
>   if( this->str )free( this->str );
>   strcpy( this->str, rhs.str );
>   return *this;
>}
>
>Thing getThing()
>{
>   Thing t("a thing");
>   return t;
>}
>
>main()
>{
>   Thing t1("");
>   t1 = getThing();             // warning
>   printf("value: %s\n", t1.str );
>
>   Thing t2 = getThing();       // no warning
>   printf("value: %s\n", t2.str );
>}

The warning is mainly caused by the non-const by-reference argument to
Thing::operator=().  For this case, the compiler is generating code
similar to this:

    Thing temp = getThing();
    t1.operator=(temp);

The warning is effectively telling you that any changes that
Thing::operator=() makes to its argument will not affect the value of
the temporary object returned by getThing().

The no warning case is a constructor.  The code

    Thing t2 = getThing();

is in this case exactly equivalent to this invocation of the copy
constructor:

    Thing t2(getThing());

In other words, Thing::operator=() is not invoked.  See the ARM,
section 12.6.1, page 284.

BTW, there are problems with your assignment operator.  What if a
Thing object is assigned to itself?

    Thing t1("oops");
    t1 = t1;

Your assignment operator first frees any memory pointed at by the
Thing::str data member, then copies directly into that freed memory.
For self-assignment, it is also copying from that same freed memory.

To fix all of these problems, you should reimplement the assignment
operator as follows:

    Thing &
    Thing::operator=(
        const Thing &rhs    // note the addition of const here
    )
    {
        if (this != &rhs) {
        if (str != 0) {
            free(str);
        }
        s = strdup(rhs.str);
        }
        return *this;
    }

I used free() and strdup() since that is what you used in your char*
constructor, but you could also use new and delete, as you probably
know.

-steve

Steve Vinoski  [phone]   [e-mail]
Distributed Object Computing Program
Hewlett-Packard, Chelmsford, MA 01824       These are my opinions.