Hi,

I just integrated the latest angelscript version in our branch (2.21.2). We we're previously using version 2.21.0. Since then, I have some unit tests which are no longer running correctly on Win32 ( haven't tested any other platforms yet ). They seem to be related to the Return Value on Stack optimisation. Here's my test code :



//------------------------------------------------------------------------
struct Castee
{
Castee()
{}

Castee(int v)
: m_v(v)
{}

Castee& operator=(const Castee& rhs)
{
m_v = rhs.m_v;
return *this;
}

int GetValue() const
{
return m_v;
}

int m_v;
};

//------------------------------------------------------------------------
struct Caster
{
Caster()
{}

Caster(int v)
: m_v(v)
{}

operator Castee() const
{
return Castee(m_v);
}

int m_v;
};

//------------------------------------------------------------------------
TEST_FIXTURE( RegistrationFixture, Test_Can_Register_An_Implicit_Value_Cast )
{
using namespace Onyx::AngelScript;

Global(m_registry)
[
ValueClass< Castee >("Castee")
[
Method("int GetValue()", &Castee::GetValue)
],

ValueClass< Caster >("Caster")
[
Constructor< Caster, int >("void f(int i)"),

ImplicitCast("Castee", &Caster::operator Castee)
]
];

m_registry.Finalize();

SetTestCode("\
int Run()\
{\
return GetValueFromCastee(Caster(5));\
}\
\
int GetValueFromCastee(Castee castee)\
{\
return castee.GetValue();\
}\
");

m_context->Prepare(GetFunctionId("int Run()"));
m_context->Execute();

CHECK_EQUAL(5, m_context->GetReturnDWord());
CHECK_ANGELSCRIPT_HAS_NOT_LOGGED_ERRORS_OR_WARNINGS(*this);
}

//------------------------------------------------------------------------
TEST_FIXTURE( RegistrationFixture, Test_Can_Register_An_Explicit_Value_Cast )
{
Global(m_registry)
[
ValueClass< Castee >("Castee")
[
Method("int GetValue()", &Castee::GetValue),
Method("Castee& opAssign(const Castee& in)", &Castee::operator=)
],

ValueClass< Caster >("Caster")
[
Constructor< Caster, int >("void f(int i)"),

ExplicitCast("Castee", &Caster::operator Castee)
]
];

m_registry.Finalize();

SetTestCode("\
int Run()\
{\
Caster caster(5);\
return GetValueFromCastee(Castee(caster));\
}\
\
int GetValueFromCastee(Castee castee)\
{\
return castee.GetValue();\
}\
");

m_context->Prepare(GetFunctionId("int Run()"));
m_context->Execute();

CHECK_EQUAL(5, m_context->GetReturnDWord());
CHECK_ANGELSCRIPT_HAS_NOT_LOGGED_ERRORS_OR_WARNINGS(*this);
}




The code of the ImplicitCast and ExplicitCast function basically registers the asBEHAVE_IMPLICIT_VALUE_CAST or asBEHAVE_VALUE_CAST functions.
Any idea ? I've traced within as_compiler.cpp, and I suspect that the return on the stack directly is to blame here. I get the following error : "[1, 65] There is no copy operator for this type available". If I define AS_OLD, I no longer get any errors.

Any help would be appreciated.

I'll look into this. I suspect you're correct in your analysis and will have a fix ready as soon as possible.

Thanks for the detailed report.

The compiler error "There is no copy operator for this type available" is not a bug in this case. It is a side effect of the changes done to return the value on the stack, because now AngelScript is require to make a copy of the returned value to an object allocated on the heap before passing it on to the GetValueFromCastee() function.

This is because AngelScript when passing an object by value to a function, the object is allocated on the heap as the receiving function will destroy the object before returning. This is something that I'll modify in a later release, but it will take a while yet.

Observe that the need for the copy operator was there before 2.21.2 as well, for example with following script:

[source]
int Run()
{
Castee castee(Caster(5));
return GetValueFromCastee(castee);
}
[/source]

In this situation AngelScript would also have to copy the castee variable to an object allocated on the heap before calling GetValueFromCastee.

Four options are available to work around the "No copy operator" error.

1. Register the opAssign method (like you did in the second UT)
2. Register the copy constructor
3. Register the type with asOBJ_POD in which case AngelScript would make a bitwise copy
4. Change the function GetValueFromCastee to take the argument as 'const Castee &in' in which case AngelScript won't make a copy



Well, that explained. There was indeed a bug with the value cast operator. The compiler was swapping the arguments when making the call to the value cast behaviour, so the 'this' pointer would actually be pointing to the return value, while the return value was pointing to the actual object. So even if you didn't get the "No copy operator" error, your test would fail in the CHECK_EQUAL(5, m_context->GetReturnDWord()); verification.

I've fixed this bug in revision 1019.



Regards,
Andreas

Thanks for the fix. I integrated it in our code branch and it has corrected lots of issues with implicit casts ( mainly in our string/wstring wrapper ).

I still have some odd behavior with the implicit value cast however, here's the thing :



int Run()
{
Caster caster(5);
return GetValueFromCastee(caster);
}

int GetValueFromCastee(const Castee& in castee)
{
return castee.GetValue();
}


This test works flawlessly. I did change the input for GetValueFromCastee as you suggested. However, if I do change the test code to :


int Run()
{
Caster caster(5);
Castee castee = caster;
return GetValueFromCastee(castee);
}

int GetValueFromCastee(const Castee& in castee)
{
return castee.GetValue();
}


I get the following errors :


[1, 84] Can't implicitly convert from 'Caster' to 'Castee'.
[1, 75] There is no copy operator for this type available.

I've double checked my code and I'm passing asBEHAVE_IMPLICIT_VALUE_CAST as the behavior correctly.

In any case, I could undef the AS_OLD now and all my tests are running correctly ( about 1400 of them ).

Pierre

You haven't registered the opAssign method for the Castee type in this case have you?

In this scenario the copy operator is necessary for the Castee type, as the compiler will first convert the Caster type to a Castee stored in a temporary variable, and then copy the temporary variable to the new variable explicitly declared and initialized with the default constructor. (yes, there is room for optimizations in the compiler here).

Interestingly, the compiler only gave the error "Can't implicitly convert from 'Caster' to 'Castee'" because you didn't have the opAssign method. This was another bug that I've now fixed in revision 1024.