Jason Goepel

Members
  • Content count

    115
  • Joined

  • Last visited

Community Reputation

797 Good

About Jason Goepel

  • Rank
    Member
  1. Left-to-right Evaluation of opEquals method.

    Even in the case where an opEquals method takes a constant reference to an argument, the compiler will create a copy.  For large objects, this seems wasteful in the common case.  Would you consider a library, compile-time setting to not ensure left-to-right evaluation?  This would allow script writers to treat "==" like a direct call to opEquals.   In C++ all the overloaded operators in a class evaluate the argument list first, effectively making them evaluate right-to-left.  I understand that AngelScript already differs from C++ in the handling of operators, and some of those differences may justify an "always left-to-right" evaluation requirement. For instance, with a single opEquals overload, AngelScript will allow comparisons using that object when it appears on either the right or left side of the "==" operator; C++ only compiles if the object is on the left.
  2. I have a bit of an odd construct in my application which is at odds with a change made in revision 2358 to the compiler.  I have registered a value type.  This value type contains configuration information for an application variable.  The assignment operator for this type modifies the underlying application variable, not the script variable.  For instance (something similar to this): MyValueType& MyValueType::opAssign(double x) { m_pApplicationVariable->SetValue(x); return *this; } MyValueType& MyValueType::opAssign(const MyValueType& x) { opAssign(x.m_pApplicationVariable->GetValue()); return *this; } I also have a comparison operator registered which compares a value to the application variable. bool MyValueType::opEquals(double x) { return m_pApplicationVariable->GetValue() == x; } My problem occurs when I try to compare an instance of this value type.  With the new left-to-right evaluation order for overloaded operators, a temporary variable is created for my value type object.  Unfortunately, the constructor isn't capable of setting up the configuration to my application variable, so when the assignment operator is called, my application generates an access violation. // declared earlier: MyValueType my_val if (my_val == 3.5) // generates... MyValueType::MyValueType() // temporary variable MyValueType& MyValueType::opAssign(const MyValueType&inout) // <-- crash bool MyValueType::opEquals(double) // comparison with temporary variable MyValueType::~MyValueType() // destruction of temporary variable How do you recommend I handle this?  Script writers cannot generate these objects directly.  Before using them they must be passed to an application function for configuration.    
  3. Assigning property without a setter

    You have only written a property "getter."  You need to write a property "setter" to get the name property to handle an assignment operator. http://www.angelcode.com/angelscript/sdk/docs/manual/doc_script_class_prop.html void set_name(TName value);
  4. JIT Compiler Suspend / BlindMindStudios

    When using the BlindMindStudio JIT compiler, functions that suspect the active context are called twice.  The first time within a JIT block, the second from the AngelScript VM, and with the stack not properly set up for the call.   I suspect this issue is a bug in BlindMindStudio's JIT compiler rather than in AngelScript, but I don't know quite enough yet to be sure.   When a context is suspended from a function called with a JIT block, it appears that the program pointer doesn't properly advance to the "next line."  The VM advances from the asBC_JitEntry instruction to the asBC_CALLSYS instruction.  If I remove the call to asIScriptContext::Suspend, then the VM doesn't ever process the asBC_CALLSYS instruction.   I have included an example below that demonstrates the problem. #include <iostream> #include "angelscript.h" #include "as_jit.h" using namespace std; void Yield(int arg) { cout << "Performing Yield... arg = " << arg << endl; asGetActiveContext()->Suspend(); } int main(int argc, char* argv[]) { asIScriptEngine* engine = asCreateScriptEngine(ANGELSCRIPT_VERSION); asCJITCompiler* jit = new asCJITCompiler(0); engine->SetEngineProperty(asEP_INCLUDE_JIT_INSTRUCTIONS, 1); engine->SetJITCompiler(jit); int r; r = engine->RegisterGlobalFunction("void Yield(int)", asFUNCTION(Yield), asCALL_CDECL); asIScriptModule* mod = engine->GetModule("module", asGM_ALWAYS_CREATE); const char* sz_code = "int main() { Yield(5); return 0; }"; mod->AddScriptSection("main", sz_code); r = mod->Build(); asIScriptContext* ctx = engine->CreateContext(); asIScriptFunction* func = mod->GetFunctionByName("main"); ctx->Prepare(func); do { r = ctx->Execute(); } while (r == asEXECUTION_SUSPENDED); int return_value = ctx->GetReturnDWord(); ctx->Release(); engine->Release(); delete jit; return return_value; }
  5. CallFuction Exceptions

    The call to CallSystemFunctionNative is wrapped in a try/catch block so that it can catch exceptions thrown by a registered function.  However, if the function is registered with asCALL_GENERIC then there is no try/catch block.  Is this intentional or an oversight?  My application has a mix of function registrations, some of them using asCALL_GENERIC, so the effect is that some exceptions thrown by a registered function are caught while others are not.
  6. Unsafe const &in

    I think my browser had some trouble with the file uploader.
  7. Unsafe const &in

    I have followed your suggestion and changed const &in to become const &inout.   I do find the automatic construction of a temporary variable passed to a const &inout parameter useful, so I have attempted a modification to make that work.  I have attached it as a patch.  It seems to work in my tests, but I am not sure if I am properly handling all cases.  If you don't incorporate this change officially, would you mind looking it over to see if I missed anything obvious to you?   Thanks
  8. Unsafe const &in

    Turning on unsafe references allows in-out references to be used for primitives and value types, meaning a function prototyped like:    void func(const ValueType &inout x)  will pass a reference to the function and not make a copy.  This works just fine, but it seems like this should also work for a constant in reference:    void func(const ValueType &in x)   However, a copy will always be made for a value type passed to an in reference, even if the in reference is constant.  This construct causes my users confusion, so I took a quick look at trying to modify the code in asCCompiler::PrepareArgument. The solution looks like it would be more complicated than simply changing what PrepareArgument does in response to an in reference.  It looks like there are a lot of checks in the compiler that check the reference type and make assumptions about what an argument is allowed to be.  Is this an accurate understanding?   It seems like the checks against the reference type in asCCompiler::MoveArgsToStack would also need to be modified, but I don't quite grasp what the new tests would be.   I would appreciate any help, thanks.   (P.S. It would also be nice to have a constant in-out reference be able to construct a temporary object if the argument passed in is found in the parameter type's constructor)  
  9. Calling Convention Bug?

    I've discovered some sort of bug in the "CallSystemFunction."  I have only encountered it in x64 Windows builds (Visual Studio 2013).   It looks like the code cleaning up the arguments incorrectly attempts to clean up the return variable.  To trigger the bug, I register a class with a destructor and register a function that returns an instance of this object and takes one as an argument, so that the argument needs to be cleaned up.  The cleanup code will then attempt to free the argument, but it will mistakenly pass a pointer to the return variable to be freed.  This pointer is not a valid heap pointer, so there is a crash.   The attached file can be added to the test project to demonstrate the problem.   The fix may be something as simple as adding the following code the the cleanup section, but I am not familiar enough with how the AngelScript compiler manages its memory and sets up function calls to make that judgement.   as_callfunc.cpp(820) // Clean up arguments const asUINT cleanCount = sysFunc->cleanArgs.GetLength(); if( cleanCount ) { args = context->m_regs.stackPointer; if( callConv >= ICC_THISCALL ) args += AS_PTR_SIZE; //** New code to address bug **/ if( descr->DoesReturnOnStack() ) args += AS_PTR_SIZE; asSSystemFunctionInterface::SClean *clean = sysFunc->cleanArgs.AddressOf(); for( asUINT n = 0; n < cleanCount; n++, clean++ ) { void **addr = (void**)&args[clean->off]; if( clean->op == 0 ) { if( *addr != 0 ) { engine->CallObjectMethod(*addr, clean->ot->beh.release); *addr = 0; } } else { asASSERT( clean->op == 1 || clean->op == 2 ); asASSERT( *addr ); if( clean->op == 2 ) engine->CallObjectMethod(*addr, clean->ot->beh.destruct); engine->CallFree(*addr); } } }
  10. "Null pointer access" putting object type in exception info

    I'm going to try and hack something together.  My immediate purpose may be satisfied by adding more exception information when the exception is set within "CallSystemFunction."  I'll let you know if I come up with anything clever.
  11. In an effort to provide more helpful error messages to my script writers, I have been looking for a way to make "null pointer access" a little more descriptive.  Most of my script writers are technical people, but their programming training is very thin.  "Null pointer access" pretty much doesn't mean anything to them.  I am hoping to provide something like "Instance of CDemoClass has not been properly initialized" or something, potentially varying based on the object.   I don't have a good way of accomplishing this within the Exception callback framework, short of analyzing the line where an exception occurred.  I have been considering adding object information, when it exists, to the context (something like m_exceptionObjectType) which could be read during the exception callback.   I know there are many instances where the object type is not readily available (like asBC_CHKREF), so I was wondering if you had any thoughts about this.   Thanks
  12. asBEHAVE_REF_CAST             ==>  opCast asBEHAVE_IMPLICIT_REF_CAST    ==>  opImplCast asBEHAVE_VALUE_CAST           ==>  opConv asBEHAVE_IMPLICIT_VALUE_CAST  ==>  opImplConv
  13. Multithreading - Creating a Module

    Thanks for the quick response, but I think there may still be a slight issue.  It seems like if two threads enter this block: ACQUIRESHARED(engineRWLock); if( lastModule && lastModule->name == name ) retModule = lastModule; else { // TODO: optimize: Improve linear search for( asUINT n = 0; n < scriptModules.GetLength(); ++n ) if( scriptModules[n] && scriptModules[n]->name == name ) { lastModule = retModule = scriptModules[n]; break; } } RELEASESHARED(engineRWLock);   Then there is the opportunity for: Thread1:  "lastModule->name == name"    true Thread2:  "scriptModules[n]->name == name"  true Thread2:  "lastModule = retModule = scriptModules[n]" Thread1:  "retModule = lastModule"   If that were to happen, then Thread1 would return the incorrect module.  It seems like "lastModule" should always be within an exclusive lock when being written to.
  14. Multithreading - Creating a Module

    Sorry.  I thought I had uploaded it.
  15. Multithreading - Creating a Module

    I think I found a gap in your solution. asCModule *asCScriptEngine::GetModule(const char *_name, bool create) { // Accept null as well as zero-length string const char *name = ""; if( _name != 0 ) name = _name; if( lastModule && lastModule->name == name ) return lastModule; lastModule = 0; ACQUIRESHARED(engineRWLock); // TODO: optimize: Improve linear search for( asUINT n = 0; n < scriptModules.GetLength(); ++n ) if( scriptModules[n] && scriptModules[n]->name == name ) { lastModule = scriptModules[n]; break; } RELEASESHARED(engineRWLock); if( lastModule ) return lastModule; if( create ) { asCModule *module = asNEW(asCModule)(name, this); if( module == 0 ) { // Out of memory return 0; } ACQUIREEXCLUSIVE(engineRWLock); scriptModules.PushLast(module); RELEASEEXCLUSIVE(engineRWLock); lastModule = module; return lastModule; } return 0; } The member variable "lastModule" should be guarded under an exclusive lock when writing.  I had one user experience a crash from a module being destroyed twice.  The problem was two threads were attempting to create new modules simultaneously.  One of the threads searched through the "scriptModules" array an found no matching module, while the other thread created a new module and saved it to "lastModule".  Then the first thread tested "lastModule" (immediately after the search), and it was not null (even though the search was not successful).  So both function calls returns the same "asIModule" pointer, setting the stage for the crash.   I have attached a patch with my solution.