Advertisement

[BUG] Problem with dictionary addon/implicit casts

Started by January 26, 2014 10:44 PM
13 comments, last by WitchLord 10 years, 9 months ago

I suspect rev 1812 "Implicit cast didn't work in initialization of global vars" broke the dictionary addon in case a global String object was passed as the agument for the get method.

    r = engine->RegisterObjectMethod("Dictionary", "void set(const String &in, ?&in)", asFUNCTION(ScriptDictionarySet_Generic), asCALL_GENERIC); assert( r >= 0 );
    r = engine->RegisterObjectMethod("Dictionary", "bool get(const String &in, ?&out) const", asFUNCTION(ScriptDictionaryGet_Generic), asCALL_GENERIC); assert( r >= 0 );

    r = engine->RegisterObjectMethod("Dictionary", "void set(const String &in, int64&in)", asFUNCTION(ScriptDictionarySetInt_Generic), asCALL_GENERIC); assert( r >= 0 );
    r = engine->RegisterObjectMethod("Dictionary", "bool get(const String &in, int64&out) const", asFUNCTION(ScriptDictionaryGetInt_Generic), asCALL_GENERIC); assert( r >= 0 );

    r = engine->RegisterObjectMethod("Dictionary", "void set(const String &in, double&in)", asFUNCTION(ScriptDictionarySetFlt_Generic), asCALL_GENERIC); assert( r >= 0 );
    r = engine->RegisterObjectMethod("Dictionary", "bool get(const String &in, double&out) const", asFUNCTION(ScriptDictionaryGetFlt_Generic), asCALL_GENERIC); assert( r >= 0 );

AS code:

String browserTableName = ''; // persistent for document's lifetime
...
// get the 'servers' query string parameter value
const Dictionary @dict = body.ownerDocument.URL.getParameters();
dict.get( 'servers', browserTableName );

browserTableName is a global var and instead of proper values it gets assigned bizarre values such as '1.12398e-315'. Debugging the code actually reveals that AS hits the following codepath in CScriptDictionary::Get(const asstring_t &key, void *value, int typeId) const:

           // We know all numbers are stored as either int64 or double, since we register overloaded functions for those
            if( it->second.typeId == asTYPEID_INT64 && typeId == asTYPEID_DOUBLE )
            {
                *(double*)value = double(it->second.valueInt);
                return true;
            }

I haven't managed to prove my suspicion correct since I'm about to hit the bed but something tells me I'm on the right track here..

Thanks. I'll look into this tomorrow.

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

Advertisement

Thanks!

Unfortunately I'm not able to reproduce this problem.

You appear to use modified versions of the dictionary and string add-ons. Can you share the code for these?

Also, can you upgrade to the latest revision 1823 and try again?

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

Ye, we using the latest revision. Our Script class uses ref-counting, the Dictionary class is pretty much the standard AS addon. Perhaps ref-counting is what breaks it, I dunno... I'll try to come up with a test case.

Btw, I was able to pinpoint the exact commit that broke it and idd it was 1812. 1811 worked fine.

Oh, and also our String class as implicit cast methods for int, float and double.. Perhaps that's the culprit here.

--

Yes, indeed, removing the implicit cast method from our String class "fixes" the problem. So to summarize, to reproduce the problem the String class has to be ref-counted and feature implicit casts to int, float or double.

Thanks, I'll try again with this additional information.

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

Advertisement

Apparently, having a double/int constructor in the string class also affects the problem.. Still investigating.

Ok, here's my messy patch: http://e4m5.net/test.patch

The string class is contained in test_addon_dictionary.cpp. The test will fail with assertion on line 20 of the test script. If you add breakpoints to objectString_CastToDouble and objectString_FactoryFromDouble, you'll notice that it hits those functions when setting and getting a value from the dict although it shouldn't be operating on doubles at all.

Apparently the compiler prefers to implicitly cast the supplied dict value from string to a double in dict.set and then tries to convert it back in dict.get

Yes, I suspected it was something like this, which is according to the rules for function overloading described in the manual.

I'll need to review this carefully. I don't think the root cause is the change done in revision 1812, but rather that fix simply uncovered a different bug that was already there in code since a long time back.

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

Yes, I suspected it was something like this, which is according to the rules for function overloading described in the manual.

I'll need to review this carefully. I don't think the root cause is the change done in revision 1812, but rather that fix simply uncovered a different bug that was already there in code since a long time back.

Ye, that's my guess too.

I don't think that exhibited behaviour matches the manual, btw. "?&in" should be of higher priority than a cast to primitive type as no conversion is needed. Btw, while we're at it, why is the prototype for dictionary::get declared as 'void set(const string &in, ?&in)' and not 'void set(const string &in, const ?&in)'?

As a workaround I'm going to disable the implicit casts for out string class for now.

This topic is closed to new replies.

Advertisement