From d6047ae651ac9166ac713f7eca018e0977abaf0c Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sat, 22 Oct 2016 12:10:19 +0200 Subject: [PATCH] - added the required code genration nodes for member function calls. This is not testable right now because finally the action function mess has come full circle. The current setup makes it impossible to call action functions from non-action functions because the needed info is local to the functions. Long avoided, this needs to be refactored now so that the different semantics for action functions are no longer needed. --- src/scripting/codegeneration/codegen.cpp | 181 +++++++++++++++++---- src/scripting/codegeneration/codegen.h | 25 ++- src/scripting/decorate/thingdef_exp.cpp | 2 +- src/scripting/decorate/thingdef_states.cpp | 4 +- src/scripting/thingdef.cpp | 2 +- src/scripting/thingdef.h | 2 +- src/scripting/zscript/zcc_compile.cpp | 2 +- 7 files changed, 180 insertions(+), 38 deletions(-) diff --git a/src/scripting/codegeneration/codegen.cpp b/src/scripting/codegeneration/codegen.cpp index a9c5d40e7..dce724a32 100644 --- a/src/scripting/codegeneration/codegen.cpp +++ b/src/scripting/codegeneration/codegen.cpp @@ -4717,12 +4717,26 @@ static bool CheckArgSize(FName fname, FArgumentList *args, int min, int max, FSc FxExpression *FxFunctionCall::Resolve(FCompileContext& ctx) { ABORT(ctx.Class); + bool error = false; - PFunction *afd = FindClassMemberFunction(ctx.Class, ctx.Class, MethodName, ScriptPosition); + PFunction *afd = FindClassMemberFunction(ctx.Class, ctx.Class, MethodName, ScriptPosition, &error); + + if (error) + { + delete this; + return nullptr; + } if (afd != nullptr) { - auto x = new FxVMFunctionCall(afd, ArgList, ScriptPosition, false); + if (ctx.Function->Variants[0].Flags & VARF_Static && !(afd->Variants[0].Flags & VARF_Static)) + { + ScriptPosition.Message(MSG_ERROR, "Call to non-static function %s from a static context", MethodName.GetChars()); + delete this; + return nullptr; + } + auto self = !(afd->Variants[0].Flags & VARF_Static)? new FxSelf(ScriptPosition) : nullptr; + auto x = new FxVMFunctionCall(self, afd, ArgList, ScriptPosition, false); ArgList = nullptr; delete this; return x->Resolve(ctx); @@ -4867,6 +4881,105 @@ FxExpression *FxFunctionCall::Resolve(FCompileContext& ctx) } +//========================================================================== +// +// +// +//========================================================================== + +FxMemberFunctionCall::FxMemberFunctionCall(FxExpression *self, FName methodname, FArgumentList *args, const FScriptPosition &pos) + : FxExpression(pos) +{ + Self = self; + MethodName = methodname; + ArgList = args; +} + +//========================================================================== +// +// +// +//========================================================================== + +FxMemberFunctionCall::~FxMemberFunctionCall() +{ + SAFE_DELETE(Self); + SAFE_DELETE(ArgList); +} + +//========================================================================== +// +// +// +//========================================================================== + +FxExpression *FxMemberFunctionCall::Resolve(FCompileContext& ctx) +{ + ABORT(ctx.Class); + SAFE_RESOLVE(Self, ctx); + + PClass *cls; + bool staticonly = false; + if (Self->ValueType->IsKindOf(RUNTIME_CLASS(PClassPointer))) + { + cls = static_cast(Self->ValueType)->ClassRestriction; + staticonly = true; + } + else if (Self->ValueType->IsKindOf(RUNTIME_CLASS(PPointer))) + { + auto ptype = static_cast(Self->ValueType)->PointedType; + if (ptype->IsKindOf(RUNTIME_CLASS(PClass))) + { + cls = static_cast(ptype); + } + else + { + ScriptPosition.Message(MSG_ERROR, "Left hand side of %s must point to a class object\n", MethodName.GetChars()); + delete this; + return nullptr; + } + } + else + { + ScriptPosition.Message(MSG_ERROR, "Invalid expression on left hand side of %s\n", MethodName.GetChars()); + delete this; + return nullptr; + } + + bool error = false; + PFunction *afd = FindClassMemberFunction(cls, cls, MethodName, ScriptPosition, &error); + if (error) + { + delete this; + return nullptr; + } + + if (afd == nullptr) + { + ScriptPosition.Message(MSG_ERROR, "Unknown function %s\n", MethodName.GetChars()); + delete this; + return nullptr; + } + if (staticonly && !(afd->Variants[0].Flags & VARF_Static)) + { + if (!ctx.Class->IsDescendantOf(cls)) + { + ScriptPosition.Message(MSG_ERROR, "Cannot call non-static function %s::%s from here\n", cls->TypeName.GetChars(), MethodName.GetChars()); + delete this; + return nullptr; + } + // If this is a qualified call to a parent class function, let it through (but this needs to disable virtual calls later.) + } + + // do not pass the self pointer to static functions. + auto self = !(afd->Variants[0].Flags & VARF_Static) ? Self : nullptr; + auto x = new FxVMFunctionCall(self, afd, ArgList, ScriptPosition, staticonly); + ArgList = nullptr; + delete this; + return x->Resolve(ctx); +} + + //========================================================================== // // FxActionSpecialCall @@ -5042,13 +5155,14 @@ ExpEmit FxActionSpecialCall::Emit(VMFunctionBuilder *build) // //========================================================================== -FxVMFunctionCall::FxVMFunctionCall(PFunction *func, FArgumentList *args, const FScriptPosition &pos, bool ownerisself) +FxVMFunctionCall::FxVMFunctionCall(FxExpression *self, PFunction *func, FArgumentList *args, const FScriptPosition &pos, bool novirtual) : FxExpression(pos) { + Self = self; Function = func; ArgList = args; EmitTail = false; - OwnerIsSelf = ownerisself; + NoVirtual = novirtual; } //========================================================================== @@ -5103,6 +5217,7 @@ VMFunction *FxVMFunctionCall::GetDirectFunction() FxExpression *FxVMFunctionCall::Resolve(FCompileContext& ctx) { CHECKRESOLVED(); + SAFE_RESOLVE_OPT(Self, ctx); bool failed = false; auto proto = Function->Variants[0].Proto; auto argtypes = proto->ArgumentTypes; @@ -5112,6 +5227,14 @@ FxExpression *FxVMFunctionCall::Resolve(FCompileContext& ctx) else if (Function->Variants[0].Flags & VARF_Method) implicit = 1; else implicit = 0; + // This should never happen. + if (Self == nullptr && !(Function->Variants[0].Flags & VARF_Static)) + { + ScriptPosition.Message(MSG_ERROR, "Call to non-static function without a self pointer"); + delete this; + return nullptr; + } + if (ArgList != NULL) { for (unsigned i = 0; i < ArgList->Size(); i++) @@ -5142,16 +5265,14 @@ FxExpression *FxVMFunctionCall::Resolve(FCompileContext& ctx) //========================================================================== // -// Assumption: This call is being generated inside a function whose a0 -// register is a self pointer. For action functions, a1 maps to stateowner -// and a2 maps to callingstate. (self, stateowner, callingstate) +// // //========================================================================== ExpEmit FxVMFunctionCall::Emit(VMFunctionBuilder *build) { assert((build->IsActionFunc && build->Registers[REGT_POINTER].GetMostUsed() >= NAP) || - (!build->IsActionFunc && build->Registers[REGT_POINTER].GetMostUsed() >= 1)); + (!build->IsActionFunc && build->Registers[REGT_POINTER].GetMostUsed() >= 1)); int count = (ArgList ? ArgList->Size() : 0); if (count == 1) @@ -5163,35 +5284,34 @@ ExpEmit FxVMFunctionCall::Emit(VMFunctionBuilder *build) } } - // Passing the caller as 'self' is a serious design mistake in DECORATE because it mixes up the types of the two actors involved. - // For ZSCRIPT 'self' is properly used for the state's owning actor, meaning we have to pass the second argument here. - // If both functions are non-action or both are action, there is no need for special treatment. - if (!OwnerIsSelf || (!!(Function->Variants[0].Flags & VARF_Action) == build->IsActionFunc)) + //if (elf || (!!(Function->Variants[0].Flags & VARF_Action) == build->IsActionFunc)) { - // Emit code to pass implied parameters + // Emit code to pass implied parameters if (Function->Variants[0].Flags & VARF_Method) - { - build->Emit(OP_PARAM, 0, REGT_POINTER, 0); - count += 1; - } + { + assert(Self != nullptr); + Self->Emit(build); + count += 1; + } if (Function->Variants[0].Flags & VARF_Action) - { - static_assert(NAP == 3, "This code needs to be updated if NAP changes"); - if (build->IsActionFunc) { - build->Emit(OP_PARAM, 0, REGT_POINTER, 1); - build->Emit(OP_PARAM, 0, REGT_POINTER, 2); + static_assert(NAP == 3, "This code needs to be updated if NAP changes"); + if (build->IsActionFunc) + { + build->Emit(OP_PARAM, 0, REGT_POINTER, 1); + build->Emit(OP_PARAM, 0, REGT_POINTER, 2); + } + else + { + int null = build->GetConstantAddress(nullptr, ATAG_GENERIC); + build->Emit(OP_PARAM, 0, REGT_POINTER | REGT_KONST, null); + build->Emit(OP_PARAM, 0, REGT_POINTER | REGT_KONST, null); + } + count += 2; } - else - { - int null = build->GetConstantAddress(nullptr, ATAG_GENERIC); - build->Emit(OP_PARAM, 0, REGT_POINTER | REGT_KONST, null); - build->Emit(OP_PARAM, 0, REGT_POINTER | REGT_KONST, null); - } - count += 2; - } } + /* else { if (build->IsActionFunc && (Function->Variants[0].Flags & VARF_Method)) @@ -5207,6 +5327,7 @@ ExpEmit FxVMFunctionCall::Emit(VMFunctionBuilder *build) I_Error("Cannot call action function from non-action functions."); } } + */ // Emit code to pass explicit parameters if (ArgList != NULL) diff --git a/src/scripting/codegeneration/codegen.h b/src/scripting/codegeneration/codegen.h index 32dc4bfbf..4f3b57c13 100644 --- a/src/scripting/codegeneration/codegen.h +++ b/src/scripting/codegeneration/codegen.h @@ -1080,6 +1080,26 @@ public: }; +//========================================================================== +// +// FxFunctionCall +// +//========================================================================== + +class FxMemberFunctionCall : public FxExpression +{ + FxExpression *Self; + FName MethodName; + FArgumentList *ArgList; + +public: + + FxMemberFunctionCall(FxExpression *self, FName methodname, FArgumentList *args, const FScriptPosition &pos); + ~FxMemberFunctionCall(); + FxExpression *Resolve(FCompileContext&); +}; + + //========================================================================== // // FxActionSpecialCall @@ -1130,12 +1150,13 @@ public: class FxVMFunctionCall : public FxExpression { bool EmitTail; - bool OwnerIsSelf; // ZSCRIPT makes the state's owner the self pointer to ensure proper type handling + bool NoVirtual; + FxExpression *Self; PFunction *Function; FArgumentList *ArgList; public: - FxVMFunctionCall(PFunction *func, FArgumentList *args, const FScriptPosition &pos, bool ownerisself); + FxVMFunctionCall(FxExpression *self, PFunction *func, FArgumentList *args, const FScriptPosition &pos, bool novirtual); ~FxVMFunctionCall(); FxExpression *Resolve(FCompileContext&); PPrototype *ReturnProto(); diff --git a/src/scripting/decorate/thingdef_exp.cpp b/src/scripting/decorate/thingdef_exp.cpp index 5be69390e..2d1d189b4 100644 --- a/src/scripting/decorate/thingdef_exp.cpp +++ b/src/scripting/decorate/thingdef_exp.cpp @@ -515,7 +515,7 @@ static FxExpression *ParseExpression0 (FScanner &sc, PClassActor *cls) args = nullptr; } - return new FxVMFunctionCall(func, args, sc, false); + return new FxVMFunctionCall(new FxSelf(sc), func, args, sc, false); } } diff --git a/src/scripting/decorate/thingdef_states.cpp b/src/scripting/decorate/thingdef_states.cpp index 11db1504d..8b75b935f 100644 --- a/src/scripting/decorate/thingdef_states.cpp +++ b/src/scripting/decorate/thingdef_states.cpp @@ -99,7 +99,7 @@ FxVMFunctionCall *DoActionSpecials(FScanner &sc, FState & state, Baggage &bag) { sc.ScriptError ("Too many arguments to %s", specname.GetChars()); } - return new FxVMFunctionCall(FindGlobalActionFunction("A_CallSpecial"), args, sc, false); + return new FxVMFunctionCall(new FxSelf(sc), FindGlobalActionFunction("A_CallSpecial"), args, sc, false); } return NULL; } @@ -571,7 +571,7 @@ FxVMFunctionCall *ParseAction(FScanner &sc, FState state, FString statestring, B { FArgumentList *args = new FArgumentList; ParseFunctionParameters(sc, bag.Info, *args, afd, statestring, &bag.statedef); - call = new FxVMFunctionCall(afd, args->Size() > 0 ? args : NULL, sc, false); + call = new FxVMFunctionCall(new FxSelf(sc), afd, args->Size() > 0 ? args : NULL, sc, false); if (args->Size() == 0) { delete args; diff --git a/src/scripting/thingdef.cpp b/src/scripting/thingdef.cpp index 7fcb0ce39..45e77b3ad 100644 --- a/src/scripting/thingdef.cpp +++ b/src/scripting/thingdef.cpp @@ -155,7 +155,7 @@ PFunction *CreateAnonymousFunction(PClass *containingclass, PType *returntype, i // //========================================================================== -PFunction *FindClassMemberFunction(PClass *selfcls, PClass *funccls, FName name, FScriptPosition &sc) +PFunction *FindClassMemberFunction(PClass *selfcls, PClass *funccls, FName name, FScriptPosition &sc, bool *error) { // Skip ACS_NamedExecuteWithResult. Anything calling this should use the builtin instead. if (name == NAME_ACS_NamedExecuteWithResult) return nullptr; diff --git a/src/scripting/thingdef.h b/src/scripting/thingdef.h index bdee476ee..51df4821e 100644 --- a/src/scripting/thingdef.h +++ b/src/scripting/thingdef.h @@ -153,7 +153,7 @@ class FxVMFunctionCall *ParseAction(FScanner &sc, FState state, FString statestr FName CheckCastKludges(FName in); void SetImplicitArgs(TArray *args, TArray *argflags, TArray *argnames, PClass *cls, DWORD funcflags); PFunction *CreateAnonymousFunction(PClass *containingclass, PType *returntype, int flags); -PFunction *FindClassMemberFunction(PClass *cls, PClass *funccls, FName name, FScriptPosition &sc); +PFunction *FindClassMemberFunction(PClass *cls, PClass *funccls, FName name, FScriptPosition &sc, bool *error); //========================================================================== // diff --git a/src/scripting/zscript/zcc_compile.cpp b/src/scripting/zscript/zcc_compile.cpp index bf3d14d06..ba038a716 100644 --- a/src/scripting/zscript/zcc_compile.cpp +++ b/src/scripting/zscript/zcc_compile.cpp @@ -2066,7 +2066,7 @@ FxExpression *ZCCCompiler::SetupActionFunction(PClassActor *cls, ZCC_TreeNode *a if (fc->Parameters == nullptr && (afd->Variants[0].Flags & VARF_Action)) { // We can use this function directly without wrapping it in a caller. - return new FxVMFunctionCall(afd, nullptr, *af, true); + return new FxVMFunctionCall(new FxSelf(*af), afd, nullptr, *af, false); } } else