1

The problem. See the below code... I was missing the return field; statement and didn't spot it for a few debug runs. I would never have thought that something like that would get past the compiler without an error! Why did it?

Description I have an parser object that uses std::tr1::function to have receive a translation algorithm on construction. The translation converts from a string to an object of type TransportableMessage, which is made of TMFields, which can hold different types. So anyway I made a factory function for the creation of the fields because the type of the field is based on a string (from the XML), and while there is only a few now, it may be that there are more in the future.

The tstring object is a typecast based on UNICODE and basically resolves to std::wstring at the minute. Also because I am using VS2008 unique_ptr is not there so I have hidden std::auto_ptr behind ScopedPtr_t to make an upgrade easier and thus more likely to happen.

namespace arc
{
    namespace util
    {

        int internalTest()
        {
            std::cout << "Any error?" << std::endl;
        }

        ScopedPtr_T<TMField> TmFieldFactory( const tstring& name, const tstring& typeName, const tstring& value )
        {
            ScopedPtr_T<TMField> field;
            int a = internalTest();
            if( typeName == _T("int") || typeName == _T("long") )
            {
                field.reset( new TMNumericField( name, boost::lexical_cast<__int64>(value) ) );
            }
            else if( typeName == _T("string") )
            {
                field.reset( new TMStringField( name, value ) );
            }
            else if( typeName == _T("timestamp") )
            {
                field.reset( new TMTimeField( name, boost::lexical_cast<__int64>(value) ) );
            }
            else
            {
                std::string info( __FILE__ " :: " __FUNCTION__ " : Unrecognized TmField type ");
                std::string type( typeName.begin(), typeName.end() );
                info += type;
                throw ARC_Exception( info.c_str() );
            }
            return field; // I WAS MISSING THIS!
        }
    }
}

Basically I would like to know if anyone else has had problems with this? I introduced the function internalTest to see if the problem was specific to scoped_ptrs but it appears that it is not. I also tried using internalTest in GCC and it returned an error. Why did it Visual Studio not flag this? Am I missing a compilation setting somewhere that I can turn on? Does the standard allow this? Apologies if this is something well known, I did google and look for answers here.

EDIT:: Calling Code - Just adding this for more context.

SharedPtr_T<TransportableMessage> strToTmConvFunc_Default_Apc7_0(const std::string& tm_str)
{
    pugi::xml_document xmlDoc;
    if( false == xmlDoc.load( tm_str.c_str() ) )
    {
        ARC_LOG(LOG_WARN, "The passed TM object was malformed. %s", tm_str.c_str() );
        throw ARC_Exception( "Malformed Transportable Message XML" );
    }

    pugi::xml_node tmBase = xmlDoc.first_child();
    std::string xmlRootName( tmBase.name() );
    if( xmlRootName.compare("TM") != 0 )
    {
        ARC_LOG(LOG_WARN, "The passed TM object was malformed. %s", tm_str.c_str() );
        throw ARC_Exception( "Malformed Transportable Message XML" );
    }

    std::string tmname = tmBase.child("N").child_value();
    std::string entity = tmBase.child("E").child_value();
    ARC_LOG(LOG_INFO, "TM message received for parsing. TM name is %s", tmname.c_str() );

    tstring t_tmname( tmname.begin(), tmname.end() );
    SharedPtr_T<TransportableMessage> tm_obj( new TransportableMessage( t_tmname, 0 ) );

    pugi::xml_node fields = tmBase.child("FS");
    for( pugi::xml_node field = fields.first_child(); field; field = field.next_sibling("field") )
    {
        tm_obj->addField( arc::util::TmFieldFactory( field.child_value("N"), field.child_value("T"), field.child_value("V") ) );
    }

    return tm_obj;
}
Dennis
  • 3,538
  • 18
  • 41
  • Might it be because you had nothing causing an instantiation of that template? – anthonyvd Aug 07 '12 at 17:58
  • 2
    Do you have [warnings turned up high](http://stackoverflow.com/a/399865/168175)? – Flexo Aug 07 '12 at 17:59
  • @pwny - Pretty sure I was instantiating the template alright, but I added the `internalTest` function to remove that from consideration. @Flexo - I hadn't. I turned it up to W4. Can't use WX because of library code. Even on W4 there is no mention of the missing return. – Dennis Aug 07 '12 at 18:04
  • Well this code must not have been compiled, because I'm pretty sure that's supposed to be an error. – anthonyvd Aug 07 '12 at 18:05
  • It would have been an error, warnings or no warnings. – Ed S. Aug 07 '12 at 18:07
  • @pwny - Definitely compiled. I ran it. It crashed. Added the `return`, re-built, re-ran, no crash. I'm stumped. Can someone else maybe try compiling something like the `internalTest` function that I have in my code, see if it is something peculiar about my system? – Dennis Aug 07 '12 at 18:08
  • What happens if you remove the `throw` statement, does it issue a warning then? In any case it really ought to be issuing [C4715](http://msdn.microsoft.com/en-us/library/6deaf4k9%28v=vs.71%29.aspx). – Adam Rosenfield Aug 07 '12 at 18:15
  • 2
    possible duplicate of [Why can you return from a non-void function without returning a value without producing a compiler error?](http://stackoverflow.com/questions/1610030/why-can-you-return-from-a-non-void-function-without-returning-a-value-without-pr) – Bo Persson Aug 07 '12 at 18:16
  • @BoPersson - Yep... Sorry I thought that I had checked thoroughly! – Dennis Aug 07 '12 at 18:24
  • @BoPersson, that makes sense - returning `0` from the function would essentially return a NULL pointer, which can certainly cause a crash when the caller tries to use the pointer. – Mark Ransom Aug 07 '12 at 18:24
  • @AdamRosenfield - No. Nothing. It really should tell about this. I think I must still be missing some compilation option. – Dennis Aug 07 '12 at 18:31
  • Have you tried `#pragma warning (error: 4715)`? – fredoverflow Aug 07 '12 at 19:01

1 Answers1

5

Falling off the body of a function without returning something is not an error, it is undefined behavior; see 6.3.3 The return statement [stmt.return] §3:

Flowing off the end of a function is equivalent to a return with no value; this results in undefined behavior in a value-returning function.

fredoverflow
  • 237,063
  • 85
  • 359
  • 638