From c3e693b507619c66728cae43a9f2f66b8098ff18 Mon Sep 17 00:00:00 2001 From: Christoph Oelckers Date: Sat, 15 Oct 2016 20:16:44 +0200 Subject: [PATCH] - added FindClassMemberFunction which retrieves a function symbol and performs some verification. - removed Self parameter from FxFunctionCall. Actual member function calls through an object require quite different handling so lumping these two together makes no sense. - added a workaround to deal with ACS_NamedExecuteWithResult to both the compiler and FindClassMemberFunction. The way the ZScript compiler sets this up means that it will call the builtin, not the actual action function, so the parser needs to do some explicit check to get past the same-named action function. - pass a proper self pointer to FxActionSpecial. Although it's still not being used, propagating design shortcuts through several function levels is a very, very bad idea. --- src/scripting/codegeneration/codegen.cpp | 35 ++++----- src/scripting/codegeneration/codegen.h | 3 +- .../codegeneration/functioncalls.cpp | 71 +------------------ src/scripting/decorate/thingdef_exp.cpp | 2 +- src/scripting/thingdef.cpp | 37 ++++++++++ src/scripting/thingdef.h | 1 + src/scripting/zscript/zcc_compile.cpp | 48 +++++++------ 7 files changed, 83 insertions(+), 114 deletions(-) diff --git a/src/scripting/codegeneration/codegen.cpp b/src/scripting/codegeneration/codegen.cpp index 4ed7128346..9cdd9c1f29 100644 --- a/src/scripting/codegeneration/codegen.cpp +++ b/src/scripting/codegeneration/codegen.cpp @@ -3630,10 +3630,9 @@ ExpEmit FxArrayElement::Emit(VMFunctionBuilder *build) // //========================================================================== -FxFunctionCall::FxFunctionCall(FxExpression *self, FName methodname, FArgumentList *args, const FScriptPosition &pos) +FxFunctionCall::FxFunctionCall(FName methodname, FArgumentList *args, const FScriptPosition &pos) : FxExpression(pos) { - Self = self; MethodName = methodname; ArgList = args; } @@ -3646,7 +3645,6 @@ FxFunctionCall::FxFunctionCall(FxExpression *self, FName methodname, FArgumentLi FxFunctionCall::~FxFunctionCall() { - SAFE_DELETE(Self); SAFE_DELETE(ArgList); } @@ -3659,9 +3657,9 @@ FxFunctionCall::~FxFunctionCall() FxExpression *FxFunctionCall::Resolve(FCompileContext& ctx) { ABORT(ctx.Class); - - PFunction *afd = dyn_cast(ctx.Class->Symbols.FindSymbol(MethodName, true)); - // Todo: More checks are needed here to make it work as expected. + + PFunction *afd = FindClassMemberFunction(ctx.Class, ctx.Class, MethodName, ScriptPosition); + if (afd != nullptr) { auto x = new FxVMFunctionCall(afd, ArgList, ScriptPosition, false); @@ -3674,14 +3672,8 @@ FxExpression *FxFunctionCall::Resolve(FCompileContext& ctx) { if (MethodName == FxFlops[i].Name) { - if (Self != NULL) - { - ScriptPosition.Message(MSG_ERROR, "Global functions cannot have a self pointer"); - delete this; - return NULL; - } FxExpression *x = new FxFlopFunctionCall(i, ArgList, ScriptPosition); - ArgList = NULL; + ArgList = nullptr; delete this; return x->Resolve(ctx); } @@ -3706,24 +3698,25 @@ FxExpression *FxFunctionCall::Resolve(FCompileContext& ctx) ScriptPosition.Message(MSG_ERROR, "Not enough parameters for '%s' (expected %d, got %d)", MethodName.GetChars(), min, paramcount); delete this; - return NULL; + return nullptr; } else if (paramcount > max) { ScriptPosition.Message(MSG_ERROR, "too many parameters for '%s' (expected %d, got %d)", MethodName.GetChars(), max, paramcount); delete this; - return NULL; + return nullptr; } - FxExpression *x = new FxActionSpecialCall(Self, special, ArgList, ScriptPosition); - ArgList = NULL; + FxExpression *self = (ctx.Function && ctx.Function->Variants[0].Flags & VARF_Method) ? new FxSelf(ScriptPosition) : nullptr; + FxExpression *x = new FxActionSpecialCall(self, special, ArgList, ScriptPosition); + ArgList = nullptr; delete this; return x->Resolve(ctx); } ScriptPosition.Message(MSG_ERROR, "Call to unknown function '%s'", MethodName.GetChars()); delete this; - return NULL; + return nullptr; } @@ -3780,6 +3773,7 @@ FxExpression *FxActionSpecialCall::Resolve(FCompileContext& ctx) CHECKRESOLVED(); bool failed = false; + SAFE_RESOLVE_OPT(Self, ctx); if (ArgList != NULL) { for (unsigned i = 0; i < ArgList->Size(); i++) @@ -3840,11 +3834,12 @@ int DecoCallLineSpecial(VMFrameStack *stack, VMValue *param, int numparam, VMRet ExpEmit FxActionSpecialCall::Emit(VMFunctionBuilder *build) { - assert(Self == NULL); unsigned i = 0; build->Emit(OP_PARAMI, abs(Special)); // pass special number - build->Emit(OP_PARAM, 0, REGT_POINTER, 0); // pass self + // fixme: This really should use the Self pointer that got passed to this class instead of just using the first argument from the function. + // Once static functions are possible, or specials can be called through a member access operator this won't work anymore. + build->Emit(OP_PARAM, 0, REGT_POINTER, 0); // pass self if (ArgList != NULL) { for (; i < ArgList->Size(); ++i) diff --git a/src/scripting/codegeneration/codegen.h b/src/scripting/codegeneration/codegen.h index d69323ce0b..7f00020c12 100644 --- a/src/scripting/codegeneration/codegen.h +++ b/src/scripting/codegeneration/codegen.h @@ -888,13 +888,12 @@ typedef TDeletingArray FArgumentList; class FxFunctionCall : public FxExpression { - FxExpression *Self; FName MethodName; FArgumentList *ArgList; public: - FxFunctionCall(FxExpression *self, FName methodname, FArgumentList *args, const FScriptPosition &pos); + FxFunctionCall(FName methodname, FArgumentList *args, const FScriptPosition &pos); ~FxFunctionCall(); FxExpression *Resolve(FCompileContext&); }; diff --git a/src/scripting/codegeneration/functioncalls.cpp b/src/scripting/codegeneration/functioncalls.cpp index d085635cd6..1c8dfb3f5e 100644 --- a/src/scripting/codegeneration/functioncalls.cpp +++ b/src/scripting/codegeneration/functioncalls.cpp @@ -39,75 +39,6 @@ // just some rough concepts for now... -//========================================================================== -// -// FindClassMemberFunction -// -// Looks for a name in a class's -// -//========================================================================== - -PFunction *FindClassMemberFunction(PClass *cls, FName name, FScriptPosition &sc) -{ - PSymbolTable *symtable; - auto symbol = cls->Symbols.FindSymbolInTable(name, symtable); - auto funcsym = dyn_cast(symbol); - - if (symbol != nullptr) - { - if (funcsym == nullptr) - { - sc.Message(MSG_ERROR, "%s is not a member function of %s", name.GetChars(), cls->TypeName.GetChars()); - } - else if (funcsym->Flags & VARF_Private && symtable != &cls->Symbols) - { - sc.Message(MSG_ERROR, "%s is declared private and not accessible", symbol->SymbolName.GetChars()); - } - else if (funcsym->Flags & VARF_Deprecated) - { - sc.Message(MSG_WARNING, "Call to deprecated function %s", symbol->SymbolName.GetChars()); - } - } - return funcsym; -} - - -//========================================================================== -// -// let's save some work down the line by analyzing the type of function -// that's being called right here and create appropriate nodes. -// A simple function call expression must be immediately resolvable in -// the context it gets compiled in, if the function name cannot be -// resolved here, it will never. -// -//========================================================================== - -FxExpression *ConvertSimpleFunctionCall(ZCC_ExprID *function, FArgumentList *args, PClass *cls, FScriptPosition &sc) -{ - // Priority is as follows: - //1. member functions of the containing class. - //2. action specials. - //3. floating point operations - //4. global builtins - - if (cls != nullptr) - { - // There is an action function ACS_NamedExecuteWithResult which must be ignored here for this to work. - if (function->Identifier != NAME_ACS_NamedExecuteWithResult) - { - { - args = ConvertNodeList(fcall->Parameters); - if (args->Size() == 0) - { - delete args; - args = nullptr; - } - - return new FxMemberunctionCall(new FxSelf(), new FxInvoker(), func, args, sc, false); - } - } - -} //========================================================================== // @@ -125,7 +56,7 @@ FxExpression *ConvertFunctionCall(ZCC_Expression *function, FArgumentList *args, switch(function->NodeType) { case AST_ExprID: - return ConvertSimpleFunctionCall(static_cast(function), args, cls, sc); + return new FxFunctionCall( ConvertSimpleFunctionCall(static_cast(function), args, cls, sc); case AST_ExprMemberAccess: return ConvertMemberCall(fcall); diff --git a/src/scripting/decorate/thingdef_exp.cpp b/src/scripting/decorate/thingdef_exp.cpp index 5dc909cd07..8b12e1c1e0 100644 --- a/src/scripting/decorate/thingdef_exp.cpp +++ b/src/scripting/decorate/thingdef_exp.cpp @@ -548,7 +548,7 @@ static FxExpression *ParseExpression0 (FScanner &sc, PClassActor *cls) while (sc.CheckToken(',')); sc.MustGetToken(')'); } - return new FxFunctionCall(NULL, identifier, args, sc); + return new FxFunctionCall(identifier, args, sc); } catch (...) { diff --git a/src/scripting/thingdef.cpp b/src/scripting/thingdef.cpp index 413999ddc2..fc281e8e8a 100644 --- a/src/scripting/thingdef.cpp +++ b/src/scripting/thingdef.cpp @@ -147,6 +147,43 @@ PFunction *CreateAnonymousFunction(PClass *containingclass, PType *returntype, i return sym; } +//========================================================================== +// +// FindClassMemberFunction +// +// Looks for a name in a class's symbol table and outputs appropriate messages +// +//========================================================================== + +PFunction *FindClassMemberFunction(PClass *selfcls, PClass *funccls, FName name, FScriptPosition &sc) +{ + // Skip ACS_NamedExecuteWithResult. Anything calling this should use the builtin instead. + if (name == NAME_ACS_NamedExecuteWithResult) return nullptr; + + PSymbolTable *symtable; + auto symbol = selfcls->Symbols.FindSymbolInTable(name, symtable); + auto funcsym = dyn_cast(symbol); + + if (symbol != nullptr) + { + if (funcsym == nullptr) + { + sc.Message(MSG_ERROR, "%s is not a member function of %s", name.GetChars(), selfcls->TypeName.GetChars()); + } + else if (funcsym->Variants[0].Flags & VARF_Private && symtable != &funccls->Symbols) + { + // private access is only allowed if the symbol table belongs to the class in which the current function is being defined. + sc.Message(MSG_ERROR, "%s is declared private and not accessible", symbol->SymbolName.GetChars()); + } + else if (funcsym->Variants[0].Flags & VARF_Deprecated) + { + sc.Message(MSG_WARNING, "Call to deprecated function %s", symbol->SymbolName.GetChars()); + } + } + // return nullptr if the name cannot be found in the symbol table so that the calling code can do other checks. + return funcsym; +} + //========================================================================== // // LoadActors diff --git a/src/scripting/thingdef.h b/src/scripting/thingdef.h index d5b189d47b..7c2b90e506 100644 --- a/src/scripting/thingdef.h +++ b/src/scripting/thingdef.h @@ -152,6 +152,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); //========================================================================== // diff --git a/src/scripting/zscript/zcc_compile.cpp b/src/scripting/zscript/zcc_compile.cpp index aee7e8b763..1561bc0967 100644 --- a/src/scripting/zscript/zcc_compile.cpp +++ b/src/scripting/zscript/zcc_compile.cpp @@ -2011,24 +2011,30 @@ FxExpression *ZCCCompiler::SetupActionFunction(PClassActor *cls, ZCC_TreeNode *a assert(fc->Function->NodeType == AST_ExprID); auto id = static_cast(fc->Function); - PFunction *afd = dyn_cast(cls->Symbols.FindSymbol(id->Identifier, true)); - if (afd != nullptr) + // We must skip ACS_NamedExecuteWithResult here, because this name both exists as an action function and as a builtin. + // The code which gets called from here can easily make use of the builtin, so let's just pass this name without any checks. + // The actual action function is still needed by DECORATE: + if (id->Identifier != NAME_ACS_NamedExecuteWithResult) { - if (fc->Parameters == nullptr && (afd->Variants[0].Flags & VARF_Action)) + PFunction *afd = dyn_cast(cls->Symbols.FindSymbol(id->Identifier, true)); + if (afd != nullptr) { - // We can use this function directly without wrapping it in a caller. - return new FxVMFunctionCall(afd, nullptr, *af, true); + 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); + } } - } - else - { - // it may also be an action special so check that first before printing an error. - if (!P_FindLineSpecial(FName(id->Identifier).GetChars())) + else { - Error(af, "%s: action function not found in %s", FName(id->Identifier).GetChars(), cls->TypeName.GetChars()); - return nullptr; + // it may also be an action special so check that first before printing an error. + if (!P_FindLineSpecial(FName(id->Identifier).GetChars())) + { + Error(af, "%s: action function not found in %s", FName(id->Identifier).GetChars(), cls->TypeName.GetChars()); + return nullptr; + } + // Action specials fall through to the code generator. } - // Action specials fall through to the code generator. } } ConvertClass = cls; @@ -2284,7 +2290,7 @@ FxExpression *ZCCCompiler::ConvertNode(ZCC_TreeNode *ast) auto fcall = static_cast(ast); assert(fcall->Function->NodeType == AST_ExprID); // of course this cannot remain. Right now nothing more complex can come along but later this will have to be decomposed into 'self' and the actual function name. auto fname = static_cast(fcall->Function)->Identifier; - return new FxFunctionCall(nullptr, fname, ConvertNodeList(fcall->Parameters), *ast); + return new FxFunctionCall(fname, ConvertNodeList(fcall->Parameters), *ast); //return ConvertFunctionCall(fcall->Function, ConvertNodeList(fcall->Parameters), ConvertClass, *ast); } @@ -2298,22 +2304,22 @@ FxExpression *ZCCCompiler::ConvertNode(ZCC_TreeNode *ast) case AST_ExprConstant: { auto cnst = static_cast(ast); - if (cnst->Type->IsKindOf(RUNTIME_CLASS(PInt))) + if (cnst->Type->IsA(RUNTIME_CLASS(PName))) + { + return new FxConstant(FName(ENamedName(cnst->IntVal)), *ast); + } + else if (cnst->Type->IsA(RUNTIME_CLASS(PInt))) { return new FxConstant(cnst->IntVal, *ast); } - else if (cnst->Type->IsKindOf(RUNTIME_CLASS(PFloat))) + else if (cnst->Type->IsA(RUNTIME_CLASS(PFloat))) { return new FxConstant(cnst->DoubleVal, *ast); } - else if (cnst->Type->IsKindOf(RUNTIME_CLASS(PString))) + else if (cnst->Type->IsA(RUNTIME_CLASS(PString))) { return new FxConstant(*cnst->StringVal, *ast); } - else if (cnst->Type->IsKindOf(RUNTIME_CLASS(PName))) - { - return new FxConstant(ENamedName(cnst->IntVal), *ast); - } else { // can there be other types?