#68 Error checking removes functionality in ClassType::GetInstance()

[Florian Richter]

I had to disable a part of ClassType::GetInstance() and add a NULL check as it didn't allow NULL anymore since some update :

    static C* GetInstance(HSQUIRRELVM vm, SQInteger idx) {
        SQUserPointer ptr = NULL;
        ClassTypeDataBase* classType = getClassTypeData(vm);
        if (classType != 0) /* type checking only done if the value has type data else it may be enum */
        {
#if !defined (SCRAT_NO_ERROR_CHECKING) && 0 // disabled as it can't handle NULL
            if (SQ_FAILED(sq_getinstanceup(vm, idx, &ptr, classType))) {
                Error::Instance().Throw(vm, Sqrat::Error::FormatTypeError(vm, idx, ClassName(vm)));
                return NULL;
            }
#else
            sq_getinstanceup(vm, idx, &ptr, 0);
            if(ptr == NULL) // detect null
            {
                return NULL;
            }
#endif
        }
...

Thanks and Sqrat rocks :)

[Wizzard]

What do you mean by "can't handle null"? If the value is not of the correct type, then this function should throw an error. GetInstance is used for type checking in Sqrat. The code should be such that GetInstance throws an error if used on a null value.

[Wizzard]

I can add a bool to the parameters of GetInstance. It would be called allowNull. I can also change all the Sqrat::Var structures with pointers involved to have allowNull be true. I think this will fix your issue. GetInstance cannot allowNull by default though or type checking will not work. So, it will default to false.

[Wizzard]

I didn't have my usual setup with me to test if it works, but I added this in commit [dc8d28]

Please get back to me saying it works and compiles properly

[Florian Richter]

Thanks! Everything works but only without this change :

     Var(HSQUIRRELVM vm, SQInteger idx) {
-        const T& instance = Var<const T&>(vm, idx).value;
+        T* instance = Var<T*>(vm, idx).value;
 #if !defined (SCRAT_NO_ERROR_CHECKING)
         if (!Error::Instance().Occurred(vm)) {
 #endif
-            value.Init(new T(instance));
+            value.Init(new T(*instance));
 #if !defined (SCRAT_NO_ERROR_CHECKING)
         }
 #endif

With it i'm hitting the following assert. (Was never hit before) :

Assertion failed: m_Ptr != NULL, file ...\sqrat\include\sqrat\sqratUtil.h, line 642

from :

    if( table.HasKey( _SC( "drawable" ) ) )
    {
        Set_Drawable( pVideo->Get_Drawable( *table.GetValue<std::string>( _SC( "drawable" ) ) ), 1 );
    }

[Wizzard]

Check whether table.GetValue<std::string>(_SC("drawable")) ever returns null
If it does, you cannot dereference it anymore as a result of this change
I did fix a bug though, but I don't think that is coincides with your issue

[Florian Richter]

Just tested it a bit more. "drawable" is always set as a string, so it's never null.

Only with the new Var() code it is null. Also happens with int.
Making it run a bit further the exception is printed :

Squirrel uncaught exception: wrong type (unknown expected, got string)
Squirrel stack trace:
[depth 1]: function [Create_Sprites()] data/scripts/main/sprites.nut line [10]
[depth 1]: Locals:
[1]: [sprite_data] table
[2]: [this] table

The table data comes from the attached file.
New_Sprite() arguments are : ( bool add_to_level, Sqrat::Table &table )
Hope this helps :)

[Wizzard]

Okay! After staring at that for 10 minutes, it came to me why that wasn't working. Fixed

(There is no Sqrat::Var<std::string*>)

[Andy Tai]

One of the current unit test fails:
I did not do detailed check to see if it is related to this issue or some other changes. Anyway, just put this as a note here for now

[atai@HPQuad sqrat.git][master]$ ../build/class_binding
[==========] Running 7 tests from 1 test case.
[----------] Global test environment set-up.
[----------] 7 tests from SqratTest
[ RUN ] SqratTest.Constructors
-1 uninitialized -2 not initialized
6 uninitialized -2 not initialized
12 test -2 not initialized
23 test2 33.5 not initialized
123 test3 133.5 second string
(instance : 0x0xe46150)
[ OK ] SqratTest.Constructors
[ RUN ] SqratTest.SimpleClassBinding
[ OK ] SqratTest.SimpleClassBinding
[ RUN ] SqratTest.InheritedClassBinding
[ OK ] SqratTest.InheritedClassBinding
[ RUN ] SqratTest.WeakRef
[ OK ] SqratTest.WeakRef
[ RUN ] SqratTest.NumConversion
[ OK ] SqratTest.NumConversion
[ RUN ] SqratTest.CEnumBinding
wrong type (integer expected, got array)
wrong type (integer expected, got string)
[ OK ] SqratTest.CEnumBinding
[ RUN ] SqratTest.NoDefaultConstructorClasses
../sqrat.git/autotools/../sqrattest/Fixture.h:110: Failure
Value of: b
Actual: 1
Expected: a
Which is: 0
../sqrat.git/autotools/../sqrattest/Fixture.h:102: Failure
Value of: a
Actual: false
Expected: true
wrong number of parameters
[ FAILED ] SqratTest.NoDefaultConstructorClasses
[----------] Global test environment tear-down
[==========] 7 tests from 1 test case ran.
[ PASSED ] 6 tests.
[ FAILED ] 1 test, listed below:
[ FAILED ] SqratTest.NoDefaultConstructorClasses

1 FAILED TEST

the failure is this part
[ RUN ] SqratTest.NoDefaultConstructorClasses
../sqrat.git/autotools/../sqrattest/Fixture.h:110: Failure
Value of: b
Actual: 1
Expected: a
Which is: 0
../sqrat.git/autotools/../sqrattest/Fixture.h:102: Failure
Value of: a
Actual: false
Expected: true
wrong number of parameters

[Wizzard]

I was throwing some exceptions in functions that shouldn't throw them
The latest commit might fix it (assuming throwing exceptions in _get and _set is OK)

[Andy Tai]

all unit tests pass. thanks