Advertisement

Script builder metadata namespace bug

Started by October 06, 2024 09:00 AM
2 comments, last by WitchLord 2 months, 2 weeks ago

The following script produces a failed assertion, because double namespaces (like namespace Foo::Bar) aren't properly scoped in script builder: (they are added, but not removed properly)

namespace Foo::Bar {
	int a = 5;
}

namespace Thing {
	[Setting]
	int b = 10;
}

I made a quick patch that fixes the problem, though I'm not sure this is how you would like it to be implemented (and it has not been battle-tested yet). Note that the diff is on an already modified version of scriptbuilder so the line numbers are gonna be off. You may just not wanna use this diff at all 😅

--- a/scriptbuilder.cpp
+++ b/scriptbuilder.cpp
@@ -396,6 +396,7 @@ int CScriptBuilder::ProcessScriptSection(const char *script, unsigned int length
 		if( token == "namespace" )
 		{
 			// Get the scope after "namespace". It can be composed of multiple nested namespaces, e.g. A::B::C
+			int n = 0;
 			do
 			{
 				do
@@ -409,8 +410,10 @@ int CScriptBuilder::ProcessScriptSection(const char *script, unsigned int length
 					if (currentNamespace != "")
 						currentNamespace += "::";
 					currentNamespace += modifiedScript.substr(pos, len);
+					n++;
 				}
 			} while (t == asTC_IDENTIFIER || (t == asTC_KEYWORD && modifiedScript.substr(pos, len) == "::"));
+			currentNamespaceStack.push_back(n);
 
 			// Search until first { is encountered
 			while( pos < modifiedScript.length() )
@@ -434,14 +437,19 @@ int CScriptBuilder::ProcessScriptSection(const char *script, unsigned int length
 		// Check if end of namespace
 		if( currentNamespace != "" && token == "}" )
 		{
-			size_t found = currentNamespace.rfind( "::" );
-			if( found != string::npos )
-			{
-				currentNamespace.erase( found );
-			}
-			else
-			{
-				currentNamespace = "";
+			assert( currentNamespaceStack.size() > 0 );
+			int n = currentNamespaceStack[currentNamespaceStack.size()];
+			currentNamespaceStack.pop_back();
+			while (n-- > 0) {
+				size_t found = currentNamespace.rfind( "::" );
+				if( found != string::npos )
+				{
+					currentNamespace.erase( found );
+				}
+				else
+				{
+					currentNamespace = "";
+				}
 			}
 			pos += len;
 			continue;

--- a/scriptbuilder.h
+++ b/scriptbuilder.h
@@ -181,6 +181,7 @@ protected:
 	std::vector<SMetadataDecl> foundDeclarations;
 	std::string currentClass;
 	std::string currentNamespace;
+	std::vector<int> currentNamespaceStack;
 
 	// Storage of metadata for global declarations
 	std::map<int, std::vector<std::string>> typeMetadataMap;

Thanks, I'll review the patch and make sure it passes the regression tests before merging it.

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

Advertisement

I've checked in this patch now

https://sourceforge.net/p/angelscript/code/2971/

AngelCode.com - game development and more - Reference DB - game developer references
AngelScript - free scripting library - BMFont - free bitmap font generator - Tower - free puzzle game

This topic is closed to new replies.

Advertisement