From: Alex Strickland on
Hi

I have this code:

// trim from start
static inline std::string & ltrim( std::string & s ) {
s.erase( s.begin(), std::find_if( s.begin(), s.end(), std::not1(
std::ptr_fun<int, int>(std::isspace) ) ) );
return s;
}
// trim from end
static inline std::string & rtrim( std::string & s ) {
s.erase( std::find_if( s.rbegin(), s.rend(), std::not1( std::ptr_fun<int,
int>(std::isspace))).base(), s.end() );
return s;
}
// trim from both ends
static inline std::string & trim( std::string & s ) {
return ltrim( rtrim( s ) );
}

This line:
std::string NoDashBaseLabel = trim( BaseLabel.substr( 0, DashPos ) );

causes this warning:
[C++ Warning] SavToRaw.cpp(656): W8030 Temporary used for parameter 's' in call
to 'trim(string &)'

If I change it to:
std::string NoDashBaseLabel = BaseLabel.substr( 0, DashPos );
NoDashBaseLabel = trim( NoDashBaseLabel );

the warning goes away. Can someone please explain why a warning is generated? I
mean, what is the problem if a Temporary is created?

Thanks
Alex


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: cgoolbis on
On Dec 15, 10:10 am, Alex Strickland <s...(a)mweb.co.za> wrote:
> Hi
>
> I have this code:
>
> // trim from start
> static inline std::string & ltrim( std::string & s ) {
> s.erase( s.begin(), std::find_if( s.begin(), s.end(), std::not1(
> std::ptr_fun<int, int>(std::isspace) ) ) );
> return s;}
>
> // trim from end
> static inline std::string & rtrim( std::string & s ) {
> s.erase( std::find_if( s.rbegin(), s.rend(), std::not1( std::ptr_fun<int,
> int>(std::isspace))).base(), s.end() );
> return s;}
>
> // trim from both ends
> static inline std::string & trim( std::string & s ) {
> return ltrim( rtrim( s ) );
>
> }
>
> This line:
> std::string NoDashBaseLabel = trim( BaseLabel.substr( 0, DashPos ) );
>
> causes this warning:
> [C++ Warning] SavToRaw.cpp(656): W8030 Temporary used for parameter 's' in call
> to 'trim(string &)'
>
> If I change it to:
> std::string NoDashBaseLabel = BaseLabel.substr( 0, DashPos );
> NoDashBaseLabel = trim( NoDashBaseLabel );
>
> the warning goes away. Can someone please explain why a warning is generated? I
> mean, what is the problem if a Temporary is created?
>
> Thanks
> Alex
>


It's not that a temporary is created, it's that the you're passing a
temporary to a function by reference that returns by reference. I
suspect if you change the functions to return by value the warning
will go away.


--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Francis Glassborow on
Alex Strickland wrote:
> Hi
>
> I have this code:
>
> // trim from start
> static inline std::string & ltrim( std::string & s ) {
> s.erase( s.begin(), std::find_if( s.begin(), s.end(), std::not1(
> std::ptr_fun<int, int>(std::isspace) ) ) );
> return s;
> }
> // trim from end
> static inline std::string & rtrim( std::string & s ) {
> s.erase( std::find_if( s.rbegin(), s.rend(), std::not1(
> std::ptr_fun<int, int>(std::isspace))).base(), s.end() );
> return s;
> }
> // trim from both ends
> static inline std::string & trim( std::string & s ) {
> return ltrim( rtrim( s ) );
> }
>
> This line:
> std::string NoDashBaseLabel = trim( BaseLabel.substr( 0, DashPos ) );
>
> causes this warning:
> [C++ Warning] SavToRaw.cpp(656): W8030 Temporary used for parameter
> 's' in call to 'trim(string &)'
>
> If I change it to:
> std::string NoDashBaseLabel = BaseLabel.substr( 0, DashPos );
> NoDashBaseLabel = trim( NoDashBaseLabel );
>
> the warning goes away. Can someone please explain why a warning is
> generated? I mean, what is the problem if a Temporary is created?
>
Because trim takes its argument by reference (rather than const
reference) This tells the compiler that trim() is allowed to modify the
argument. But if it is bound to a temporary the modification may be
lost. In this case it gets worse because you return the modified version
by reference but the thing being referred to is about to go away.

It isn't just a warning but identifies a genuine error in your code.

--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Alex Strickland on
Francis Glassborow wrote:

>> // trim from both ends
>> static inline std::string & trim( std::string & s ) {
>> return ltrim( rtrim( s ) );
>> }

> It isn't just a warning but identifies a genuine error in your code.

Thank you. I have changed the above to:

static inline std::string & trim( std::string s ) {

Compiles and works, but doesn't seem optimal?

Regards
Alex



--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]

From: Seungbeom Kim on
Alex Strickland wrote:
> Francis Glassborow wrote:
>
>>> // trim from both ends
>>> static inline std::string & trim( std::string & s ) {
>>> return ltrim( rtrim( s ) );
>>> }
>
>> It isn't just a warning but identifies a genuine error in your code.
>
> Thank you. I have changed the above to:
>
> static inline std::string & trim( std::string s ) {
>
> Compiles and works, but doesn't seem optimal?

Now you're probably returning a reference to the function parameter s,
which goes out of scope and gets destroyed at the end of the function,
and you get a dangling reference.

Unless you're returning a reference to an object whose lifetime is
independent of the function call (such as a data member), you should
return by value.

--
Seungbeom Kim

[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]