Jump to content

  • Log In with Google      Sign In   
  • Create Account

Sir Ementaler

Member Since 09 Mar 2013
Offline Last Active Today, 06:08 PM

#5290775 Various unexpected behaviors of AngelScript 2.31.0

Posted by on 09 May 2016 - 05:26 AM

Looks like the "auto foo = null;" crash may still occur when declared in the global scope.

#5282810 Various unexpected behaviors of AngelScript 2.31.0

Posted by on 23 March 2016 - 12:35 AM

Thanks for explaining; sounds very reasonable. No, I don't need it disabled, given that it's intended to be permitted syntax, possesses well-defined behavior, and has legitimate use cases. That's sufficient to rename it from a bug to a feature in my book.

#5282658 Various unexpected behaviors of AngelScript 2.31.0

Posted by on 22 March 2016 - 10:07 AM

Ternary conditional operator applied to two null handles:

class foo {}
void main() {
	foo@ bar = true ? null : null;
Outcome: Compile error: Can't implicitly convert from '<null handle>' to 'foo@&'.
Expected outcome: Compiles, bar is initialized to null.
The expression has little use outside of temporary code / debugging, but it's reasonable to expect it to compile.
Auto initialized to null:
void main() {
	auto foo = null;
Outcome: Null pointer access in asCDataType::CanBeInstantiated, crash.
Expected outcome: Compile error or foo is of null handle type.

Again, there is typically no reason to do this, and it's not necessary that it compile, but ideally the engine would not crash when given incorrect code.


Return reference to void:

void& f() {
	return f();
Outcome: Compiles.
Expected outcome: Compile error: Data type cannot be reference to void.
Not very dangerous, because the only thing such a function can return is the result of another function that return void&, but this shouldn't be allowed to compile nonetheless.
Take reference to void:
void f(void &in, void &in) {}
void g() {}
void main() {
	f(g(), g());
Outcome: Assertion failure in asCCompiler::ConvertToVariable.
Expected outcome: Compile error: Data type cannot be reference to void.
A more malicious version of the previous issue.
Void value used as argument of function that takes no arguments.
void f() {}
void main() {
Outcome: Compiles, calls f twice.
Expected outcome: Compile error: Function f does not take arguments.

This is a bit of a special case, because I notice it's akin to C function definitions where functions that take no parameters are declared to take void, but wherever this kind of syntax is encountered, it appears to me that it's almost certain it's in error. Perhaps minimally a warning would be in place in this case.

#5243218 Possible dictionary optimization

Posted by on 28 July 2015 - 11:46 AM

Hi, I was skimming through scriptdictionary.cpp today to see some implementation details, and I couldn't help but notice the quite wordy implementation of CScriptDictionary::operator[]. Let me quote:

CScriptDictValue *CScriptDictionary::operator[](const string &key)
	// Return the existing value if it exists, else insert an empty value
	map<string, CScriptDictValue>::iterator it;
	it = dict.find(key);
	if( it == dict.end() )
		it = dict.insert(map<string, CScriptDictValue>::value_type(key, CScriptDictValue())).first;
	return &it->second;

It came to my attention that this is essentially a lengthy rewording of the following code:

CScriptDictValue *CScriptDictionary::operator[](const string &key)
	return &dict[key];

(see http://en.cppreference.com/w/cpp/container/map/operator_at or http://www.cplusplus.com/reference/map/map/operator[]/ for reference for std::map operator[]). I wanted to see if something escapes my attention so I tested performance of both of these implementations with the following script:

void test() {
	array<string> keys;
	for (uint i = 0; i < 100000; i++)
		keys.insertLast(formatInt(i, "", 10));

	uint insertionTime = 0, accessTime = 0;
	for (uint i = 0; i < 100; i++) {
		dictionary d;

		uint time1 = GetSystemTime();

		for (uint j = 0; j < 100000; j++)
			d[keys[j]] = j;

		uint time2 = GetSystemTime();

		for (uint j = 0; j < 100000; j++)
			if (uint(d[keys[j]]) != j)
				return 0;

		uint time3 = GetSystemTime();

		insertionTime += time2 - time1;
		accessTime += time3 - time2;
	Print("Insertion time: " + insertionTime + " ms\nAccess time: " + accessTime + " ms\n");

Using AngelScript compiled in MSVC 2013 I obtained the following results:


Current implementation:

In Release mode: Insertion time: 18334 ms; Access time: 11434 ms

In Debug mode: Insertion time: 846300 ms; Access time: 300030 ms

Proposed implementation:

In Release mode: Insertion time: 15181 ms; Access time: 12121 ms

In Debug mode: Insertion time: 577720 ms; Access time: 271790 ms


I.e., in all situations except for Release mode access the simpler implementation was faster. It may be a bit of a tradeoff but looks worth it. I don't know how other compilers would react though. Either way I thought you may be interested.

#5191276 Including namespace on GetTypeDeclaration()

Posted by on 05 November 2014 - 01:52 AM

I had a similar issue recently involving asIScriptFunction::GetDeclaration. While personally I certainly don't mind it explicitly stating the global namespace, it appeared as if asIScriptModule::GetFunctionByDecl was unable to interpret it. That means that neither including nor omitting the namespace always allows for identification of a function. Additionally, in this case the redundant "::" could prove more difficult to find and remove if one were to do that by hand, because it doesn't directly precede the string (and there could potentially be more occurrences of it, e.g. consider "foo::bar ::function(foo::bar)"). What I ended up doing was

asFunction->GetDeclaration(true, asFunction->GetNamespace()[0] != '\0', false)

which, honestly, looks like a terrible hack, but works. Either way, if not getting rid of the explicit preceding scope resolution operator, I would suggest at least modifying GetFunctionByDecl and possibly other similar functions accordingly, as they should most certainly be able to interpret declarations returned by other library functions.