- changed VMValue to handle strings by reference.

This makes VMValue a real POD type with no hacky overloads and eliminates a lot of destructor code in all places that call a VM function. Due to the way this had to be handled, none of these destructors could be skipped because any value could have been a string.
This required some minor changes in functions that passed a temporary FString into the VM to ensure that the temporary object lives long enough to be handled. The code generator had already been changed to deal with this in a previous commit.
This is easily offset by the code savings and reduced maintenance needs elsewhere.

src/m_cheat.cpp

@@ -630,7 +630,8 @@ void cht_Give (player_t *player, const char *name, int amount)
 
 	IFVIRTUALPTR(player->mo, APlayerPawn, CheatGive)
 	{
-		VMValue params[3] = { player->mo, FString(name), amount };
+		FString namestr = name;
+		VMValue params[3] = { player->mo, &namestr, amount };
 		GlobalVMStack.Call(func, params, 3, nullptr, 0);
 	}
 }
@@ -641,7 +642,8 @@ void cht_Take (player_t *player, const char *name, int amount)
 
 	IFVIRTUALPTR(player->mo, APlayerPawn, CheatTake)
 	{
-		VMValue params[3] = { player->mo, FString(name), amount };
+		FString namestr = name;
+		VMValue params[3] = { player->mo, &namestr, amount };
 		GlobalVMStack.Call(func, params, 3, nullptr, 0);
 	}
 }

src/menu/menu.cpp

@@ -1118,7 +1118,8 @@ DMenuItemBase * CreateOptionMenuItemStaticText(const char *name, bool v)
 {
 	auto c = PClass::FindClass("OptionMenuItemStaticText");
 	auto p = c->CreateNew();
-	VMValue params[] = { p, FString(name), v };
+	FString namestr = name;
+	VMValue params[] = { p, &namestr, v };
 	auto f = dyn_cast<PFunction>(c->Symbols.FindSymbol("Init", false));
 	GlobalVMStack.Call(f->Variants[0].Implementation, params, countof(params), nullptr, 0);
 	return (DMenuItemBase*)p;
@@ -1128,7 +1129,8 @@ DMenuItemBase * CreateOptionMenuItemJoyConfigMenu(const char *label, IJoystickCo
 {
 	auto c = PClass::FindClass("OptionMenuItemJoyConfigMenu");
 	auto p = c->CreateNew();
-	VMValue params[] = { p, FString(label), joy };
+	FString namestr = label;
+	VMValue params[] = { p, &namestr, joy };
 	auto f = dyn_cast<PFunction>(c->Symbols.FindSymbol("Init", false));
 	GlobalVMStack.Call(f->Variants[0].Implementation, params, countof(params), nullptr, 0);
 	return (DMenuItemBase*)p;
@@ -1138,7 +1140,8 @@ DMenuItemBase * CreateOptionMenuItemSubmenu(const char *label, FName cmd, int ce
 {
 	auto c = PClass::FindClass("OptionMenuItemSubmenu");
 	auto p = c->CreateNew();
-	VMValue params[] = { p, FString(label), cmd.GetIndex(), center };
+	FString namestr = label;
+	VMValue params[] = { p, &namestr, cmd.GetIndex(), center };
 	auto f = dyn_cast<PFunction>(c->Symbols.FindSymbol("Init", false));
 	GlobalVMStack.Call(f->Variants[0].Implementation, params, countof(params), nullptr, 0);
 	return (DMenuItemBase*)p;
@@ -1148,7 +1151,8 @@ DMenuItemBase * CreateOptionMenuItemControl(const char *label, FName cmd, FKeyBi
 {
 	auto c = PClass::FindClass("OptionMenuItemControlBase");
 	auto p = c->CreateNew();
-	VMValue params[] = { p, FString(label), cmd.GetIndex(), bindings };
+	FString namestr = label;
+	VMValue params[] = { p, &namestr, cmd.GetIndex(), bindings };
 	auto f = dyn_cast<PFunction>(c->Symbols.FindSymbol("Init", false));
 	GlobalVMStack.Call(f->Variants[0].Implementation, params, countof(params), nullptr, 0);
 	return (DMenuItemBase*)p;
@@ -1158,7 +1162,8 @@ DMenuItemBase * CreateListMenuItemPatch(double x, double y, int height, int hotk
 {
 	auto c = PClass::FindClass("ListMenuItemPatchItem");
 	auto p = c->CreateNew();
-	VMValue params[] = { p, x, y, height, tex.GetIndex(), FString(char(hotkey)), command.GetIndex(), param };
+	FString keystr = FString(char(hotkey));
+	VMValue params[] = { p, x, y, height, tex.GetIndex(), &keystr, command.GetIndex(), param };
 	auto f = dyn_cast<PFunction>(c->Symbols.FindSymbol("InitDirect", false));
 	GlobalVMStack.Call(f->Variants[0].Implementation, params, countof(params), nullptr, 0);
 	return (DMenuItemBase*)p;
@@ -1168,7 +1173,9 @@ DMenuItemBase * CreateListMenuItemText(double x, double y, int height, int hotke
 {
 	auto c = PClass::FindClass("ListMenuItemTextItem");
 	auto p = c->CreateNew();
-	VMValue params[] = { p, x, y, height, FString(char(hotkey)), text, font, int(color1.d), int(color2.d), command.GetIndex(), param };
+	FString keystr = FString(char(hotkey));
+	FString textstr = text;
+	VMValue params[] = { p, x, y, height, &keystr, &textstr, font, int(color1.d), int(color2.d), command.GetIndex(), param };
 	auto f = dyn_cast<PFunction>(c->Symbols.FindSymbol("InitDirect", false));
 	GlobalVMStack.Call(f->Variants[0].Implementation, params, countof(params), nullptr, 0);
 	return (DMenuItemBase*)p;
@@ -1191,7 +1198,8 @@ bool DMenuItemBase::SetString(int i, const char *s)
 {
 	IFVIRTUAL(DMenuItemBase, SetString)
 	{
-		VMValue params[] = { (DObject*)this, i, FString(s) };
+		FString namestr = s;
+		VMValue params[] = { (DObject*)this, i, &namestr };
 		int retval;
 		VMReturn ret(&retval);
 		GlobalVMStack.Call(func, params, countof(params), &ret, 1, nullptr);

src/menu/menudef.cpp

@@ -386,12 +386,15 @@ static void ParseListMenuBody(FScanner &sc, DListMenuDescriptor *desc)
 					}
 					auto TypeCVar = NewPointer(NewNativeStruct("CVar", nullptr));
 
+					// Note that this array may not be reallocated so its initial size must be the maximum possible elements.
+					TArray<FString> strings(args.Size());
 					for (unsigned i = start; i < args.Size(); i++)
 					{
 						sc.MustGetString();
 						if (args[i] == TypeString)
 						{
-							params.Push(FString(sc.String));
+							strings.Push(sc.String);
+							params.Push(&strings.Last());
 						}
 						else if (args[i] == TypeName)
 						{
@@ -751,12 +754,16 @@ static void ParseOptionMenuBody(FScanner &sc, DOptionMenuDescriptor *desc)
 
 					params.Push(0);
 					auto TypeCVar = NewPointer(NewNativeStruct("CVar", nullptr));
+
+					// Note that this array may not be reallocated so its initial size must be the maximum possible elements.
+					TArray<FString> strings(args.Size());
 					for (unsigned i = 1; i < args.Size(); i++)
 					{
 						sc.MustGetString();
 						if (args[i] == TypeString)
 						{
-							params.Push(FString(sc.String));
+							strings.Push(sc.String);
+							params.Push(&strings.Last());
 						}
 						else if (args[i] == TypeName)
 						{

src/menu/messagebox.cpp

@@ -67,7 +67,8 @@ DMenu *CreateMessageBoxMenu(DMenu *parent, const char *message, int messagemode,
 {
 	auto c = PClass::FindClass("MessageBoxMenu");
 	auto p = c->CreateNew();
-	VMValue params[] = { p, parent, FString(message), messagemode, playsound, action.GetIndex(), reinterpret_cast<void*>(handler) };
+	FString namestr = message;
+	VMValue params[] = { p, parent, &namestr, messagemode, playsound, action.GetIndex(), reinterpret_cast<void*>(handler) };
 
 	auto f = dyn_cast<PFunction>(c->Symbols.FindSymbol("Init", false));
 	GlobalVMStack.Call(f->Variants[0].Implementation, params, countof(params), nullptr, 0);

src/p_acs.cpp

@@ -5368,6 +5368,9 @@ static int ScriptCall(unsigned argc, int32_t *args)
 		{
 			I_Error("ACS call to non-static script function %s.%s", clsname, funcname);
 		}
+
+		// Note that this array may not be reallocated so its initial size must be the maximum possible elements.
+		TArray<FString> strings(argc);
 		TArray<VMValue> params;
 		for (unsigned i = 2; i < argc; i++)
 		{
@@ -5395,7 +5398,8 @@ static int ScriptCall(unsigned argc, int32_t *args)
 			}
 			else if (argtype == TypeString)
 			{
-				params.Push(FBehavior::StaticLookupString(args[i]));
+				strings.Push(FBehavior::StaticLookupString(args[i]));
+				params.Push(&strings.Last());
 			}
 			else if (argtype == TypeSound)
 			{

src/scripting/vm/vm.h

@@ -419,29 +419,21 @@ struct VMValue
 		double f;
 		struct { int pad[3]; VM_UBYTE Type; };
 		struct { int foo[4]; } biggest;
+		const FString *sp;
 	};
 
 	// Unfortunately, FString cannot be used directly.
 	// Fortunately, it is relatively simple.
-	FString &s() { return *(FString *)&a; }
+	const FString &s() const { return *sp; }
-	const FString &s() const { return *(FString *)&a; }
 
 	VMValue()
 	{
 		a = NULL;
 		Type = REGT_NIL;
 	}
-	~VMValue()
-	{
-		Kill();
-	}
 	VMValue(const VMValue &o)
 	{
 		biggest = o.biggest;
-		if (Type == REGT_STRING)
-		{
-			::new(&s()) FString(o.s());
-		}
 	}
 	VMValue(int v)
 	{
@@ -453,14 +445,12 @@ struct VMValue
 		f = v;
 		Type = REGT_FLOAT;
 	}
-	VMValue(const char *s)
+	VMValue(const char *s) = delete;
+	VMValue(const FString &s) = delete;
+
+	VMValue(const FString *s)
 	{
-		::new(&a) FString(s);
+		sp = s;
-		Type = REGT_STRING;
-	}
-	VMValue(const FString &s)
-	{
-		::new(&a) FString(s);
 		Type = REGT_STRING;
 	}
 	VMValue(DObject *v)
@@ -483,68 +473,31 @@ struct VMValue
 	}
 	VMValue &operator=(const VMValue &o)
 	{
-		if (o.Type == REGT_STRING)
-		{
-			if (Type == REGT_STRING)
-			{
-				s() = o.s();
-			}
-			else
-			{
-				new(&s()) FString(o.s());
-				Type = REGT_STRING;
-			}
-		}
-		else
-		{
-			Kill();
 		biggest = o.biggest;
-		}
 		return *this;
 	}
 	VMValue &operator=(int v)
 	{
-		Kill();
 		i = v;
 		Type = REGT_INT;
 		return *this;
 	}
 	VMValue &operator=(double v)
 	{
-		Kill();
 		f = v;
 		Type = REGT_FLOAT;
 		return *this;
 	}
-	VMValue &operator=(const FString &v)
+	VMValue &operator=(const FString *v)
 	{
-		if (Type == REGT_STRING)
+		sp = v;
-		{
-			s() = v;
-		}
-		else
-		{
-			::new(&s()) FString(v);
 		Type = REGT_STRING;
-		}
-		return *this;
-	}
-	VMValue &operator=(const char *v)
-	{
-		if (Type == REGT_STRING)
-		{
-			s() = v;
-		}
-		else
-		{
-			::new(&s()) FString(v);
-			Type = REGT_STRING;
-		}
 		return *this;
 	}
+	VMValue &operator=(const FString &v) = delete;
+	VMValue &operator=(const char *v) = delete;
 	VMValue &operator=(DObject *v)
 	{
-		Kill();
 		a = v;
 		atag = ATAG_OBJECT;
 		Type = REGT_POINTER;
@@ -584,13 +537,6 @@ struct VMValue
 		// FIXME
 		return 0;
 	}
-	void Kill()
-	{
-		if (Type == REGT_STRING)
-		{
-			s().~FString();
-		}
-	}
 };
 
 class VMFunction

src/scripting/vm/vmexec.h

@@ -583,7 +583,7 @@ begin:
 					break;
 				case REGT_STRING:
 					assert(C < f->NumRegS);
-					::new(param) VMValue(reg.s[C]);
+					::new(param) VMValue(&reg.s[C]);
 					break;
 				case REGT_STRING | REGT_ADDROF:
 					assert(C < f->NumRegS);
@@ -591,7 +591,7 @@ begin:
 					break;
 				case REGT_STRING | REGT_KONST:
 					assert(C < sfunc->NumKonstS);
-					::new(param) VMValue(konsts[C]);
+					::new(param) VMValue(&konsts[C]);
 					break;
 				case REGT_POINTER:
 					assert(C < f->NumRegA);
@@ -707,10 +707,7 @@ begin:
 				stack->PopFrame();
 			}
 			assert(numret == C && "Number of parameters returned differs from what was expected by the caller");
-			for (b = B; b != 0; --b)
+			f->NumParam -= B;
-			{
-				reg.param[--f->NumParam].~VMValue();
-			}
 			pc += C;			// Skip RESULTs
 		}
 		NEXTOP;

src/scripting/vm/vmframe.cpp

@@ -384,12 +384,6 @@ VMFrame *VMFrameStack::PopFrame()
 	{
 		(regs++)->~FString();
 	}
-	// Free any parameters this frame left behind.
-	VMValue *param = frame->GetParam();
-	for (int i = frame->NumParam; i != 0; --i)
-	{
-		(param++)->~VMValue();
-	}
 	VMFrame *parent = frame->ParentFrame;
 	if (parent == NULL)
 	{

src/scripting/zscript/zcc_compile.cpp

@@ -50,6 +50,8 @@
 #include "gdtoa.h"
 #include "backend/vmbuilder.h"
 
+FSharedStringArena VMStringConstants;
+
 static int GetIntConst(FxExpression *ex, FCompileContext &ctx)
 {
 	ex = new FxIntCast(ex, false);
@@ -2555,7 +2557,8 @@ void ZCCCompiler::CompileFunction(ZCC_StructWork *c, ZCC_FuncDeclarator *f, bool
 									break;
 
 								case REGT_STRING:
-									vmval[0] = cnst->GetValue().GetString();
+									// We need a reference to something permanently stored here.
+									vmval[0] = VMStringConstants.Alloc(cnst->GetValue().GetString());
 									break;
 
 								default: