JamesBoer / Jinx

Embeddable scripting language for real-time applications
https://jamesboer.github.io/Jinx/
MIT License
305 stars 11 forks source link

non-local functions added to internal function list twice #21

Closed incanus closed 2 years ago

incanus commented 2 years ago

I noticed that if you declare a public or private function in a script, both the JxRuntime::CreateScript() step (and other compilations) and then the actual JxScript::Execute() step add the function signature to the script library's internal JxLibrary::m_functionList. I've confirmed this with a debugger and breaking on JxLibrary::RegisterFunctionSignature(), where both functions hit the breakpoint.

Compilation backtrace:

Jinx::Impl::Library::RegisterFunctionSignature
Jinx::Impl::Parser::ParseFunctionDefinition
Jinx::Impl::Parser::ParseStatement
Jinx::Impl::Parser::ParseScript
Jinx::Impl::Parser::Execute
Jinx::Impl::Runtime::Compile
Jinx::Impl::Runtime::Compile
Jinx::Impl::Runtime::CreateScript

Execution backtrace:

Jinx::Impl::Library::RegisterFunctionSignature
Jinx::Impl::Script::Execute

Though I don't see any side effects to this, it seems like the simple addition of a call to m_library->FunctionSignatureExists(signature) inside of JxScript.cpp's handling of Opcode::Function takes care of this.

The whole reason this happens is because in that handling, once the scope is determined to not be VisibilityType::Local, the function gets added to the library.

JamesBoer commented 2 years ago

Thanks for the report. I decided to silently perform a check inside RegisterFunctionSignature(). It's a tiny bit of code duplication, but it lets me avoid having to take a lock twice. Now fixed in v1.3.9