Home Dashboard Directory Help
Search

C++ compiler: Temporary object created when it should not, thus making code that worked in VC9 not work anymore! by Patrik E.1


Status: 

Closed
 as Fixed Help for as Fixed


1
0
Sign in
to vote
Type: Bug
ID: 611359
Opened: 10/7/2010 2:55:29 AM
Access Restriction: Public
0
Workaround(s)
view
1
User(s) can reproduce this bug

Description

Please just copy the code below into a Visual Studio 2010 C++ console application solution.
Read the comments to the code parts and do the debugging.
I hope you can reproduce what I get.
If not, then ask me back and I will try to formulate in words.

Edit: I attached a VS2010 solution for beeing able to easily reproduce this problem!

#include "stdafx.h"
#include <map>
#include <string>
#include <assert.h>


// The following belongs to the code that DOES work under VC10!
struct SStruct
{
            SStruct()
                :    m_bIsModified(false)
                {
                }

    bool    m_bIsModified;
};

void DoModifyStructPrivate(    SStruct&    the_rStruct,
                            SStruct*    the_pStruct)
{
    assert(!the_pStruct || the_pStruct == &the_rStruct);
    the_rStruct.m_bIsModified = true;
    assert(the_rStruct.m_bIsModified);
}

void DoModifyStructPublic(SStruct* the_pStruct = 0)
{
    DoModifyStructPrivate(    the_pStruct ? *the_pStruct : SStruct(),
                            the_pStruct ? the_pStruct : 0);
}
// End of 'The following belongs to the code that DOES work under VC10!'



// The following belongs to the code that does NOT work under VC10!
struct SStructFailure
{
            SStructFailure()
                {
                }

    std::map<unsigned int, std::string> m_Map;
};

void DoModifyStructFailurePrivate(    SStructFailure& the_rStructFailure,
                                    SStructFailure* the_pStructFailure)
{
    // The following assert would fail in VC10!
    //assert(!the_pStructFailure || the_pStructFailure == &the_rStructFailure);
    the_rStructFailure.m_Map.insert(std::make_pair(500, "Five hundred"));
    assert(the_rStructFailure.m_Map.size());
}

void DoModifyStructFailurePublic(SStructFailure* the_pStructFailure = 0)
{
    // the following works in VC9 and would even work in VC10!
    /*
    SStructFailure a_StructFailure;
    DoModifyStructFailurePrivate(    the_pStructFailure ? *the_pStructFailure : a_StructFailure,
                                    the_pStructFailure ? the_pStructFailure : 0);
    */

    // the following does NOT work as expected in VC10, it does in VC9!
    DoModifyStructFailurePrivate(    the_pStructFailure ? *the_pStructFailure : SStructFailure(),
                                    the_pStructFailure ? the_pStructFailure : 0);
}
// End of 'The following belongs to the code that does NOT work under VC10!'



int _tmain(int argc, _TCHAR* argv[])
{
    // Code with 'simple' struct, that works the same in VC9 and VC10!
    SStruct a_StructToModify;
    DoModifyStructPublic(&a_StructToModify);
    assert(a_StructToModify.m_bIsModified);


    // Code with struct containing a map, that works in VC9 BUT DOES NOT in VC10!
    SStructFailure a_StructFailureToModify;
    DoModifyStructFailurePublic(&a_StructFailureToModify);
    assert(a_StructFailureToModify.m_Map.size());

    return 0;
}
Details
Sign in to post a comment.
Posted by Patrik E.1 on 2/7/2011 at 12:38 AM
Thanks for looking into and fixing this issue!
Posted by Microsoft on 2/4/2011 at 6:09 PM
Hi,
    The compiler should always create the temporary according to the standard.
    It is a bug in our compiler that simple struct has different behavior from SStructFailure. A fix for this issue has been checked into the compiler sources. The fix should show up in the next release of Visual C++.

    To detect class rvalue -> lvalue conversion, you can use:

#pragma warning(1: 4238)
#pragma warning(1: 4239)

Xiang Fan
Visual C++ Team
Posted by Patrik E.1 on 11/30/2010 at 7:13 AM
I do NOT agree that this 'case' is resolved!

I thought so that changes in the C++0x standard and their correct implementation are 'responsible' for the new behavior, but for me it does NOT explain, why the code part dealing with the simple struct containing just a bool member works as if nothing would have been changed in the C++0x standard.
I know that in the sample code I use a non standard extension (rvalue where lvalue would be required).
This is allowed by your compiler *1 and therefore it should work the SAME independent of the struct (resp. its internal assembling) which is passed to the conditional operator.

One thing I found out is that as soon as I implement a copy constructor and an assignment operator for the simple struct containing the bool member than the conditional operator behaves the same (also creates a temporary object) as it does when dealing with the struct SStructFailure.

And yes, for me it is not a big problem to rewrite our code so that it just doesn't make use of the non standard extension and thus make the code work again as expected.
But my concerns are that some code of 3rd party libraries we use would compile this way.
At least a new warning should be issued in such a case, because if there was not a long chain of fortunate coincidences I would never have found out about this new behavior, which in our case did not lead to a crash but to some unoptimized behavior of our code.

I will remove the old solution I once attached and add a new one where the effect of implementing the user defined copy constructor for the simple struct can be more easily be reproduced!

Thanks,
Patrik


*1:
In my case I wouldn't be even able to turn off non standard extensions, because then microsoft stuff in the headers I need for my MFC application wouldn't compile anymore...
Posted by Microsoft on 11/29/2010 at 3:52 PM
Hi Patrik:
    This behavior change is related to the recent change in the C++0x standard. It can be found in 5.16 of the C++0x standard (http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2010/n3126.pdf).

    When the second operand of the conditional operator is an rvalue, C++ standard 2003 says:
    E1 is changed to an rvalue of type T2 that still refers to the original source class object (or the appropriate subobject thereof).

    It is changed to the following statement in C++0x:
    E1 is changed to a prvalue of type T2 by copy-initializing a temporary of type T2 from E1 and using that temporary as the converted operand.

    (Here, E1 refers to the first operand of the contional operator and T2 refers to the type of the second operand)
Posted by Patrik E.1 on 11/16/2010 at 3:12 AM
Any news on this?
Posted by Microsoft on 10/7/2010 at 10:25 PM
Thanks for your feedback.
We are routing this issue to the appropriate group within the Visual Studio Product Team for triage and resolution. These specialized experts will follow-up with your issue.
Posted by Microsoft on 10/7/2010 at 3:20 AM
Thank you for your feedback, we are currently reviewing the issue you have submitted. If this issue is urgent, please contact support directly(http://support.microsoft.com)
Sign in to post a workaround.
File Name Submitted By Submitted On File Size  
VC2010BugInConditionalOperatorIfUsedInNonStandardWay.zip 11/30/2010 2.39 MB