From 67886990150f4b2352358fc18cbddca73e27bf8a Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 25 Oct 2024 21:01:30 -0700 Subject: [PATCH] Allow natives to return structs and arrays. Natives can now return enum structs and fixed-size arrays. The address is passed as a hidden first parameter. For example: native Struct DoStuff(int x, int y); Will have: params[0] = 3 params[1] = the address of the output struct params[2] = x params[3] = y The native MUST return params[1] on success, otherwise, behavior is undefined. It is recommended that when defining C++ definitions of SourcePawn structs, to use `#pragma pack(push, 1)` and `#pragma pack(pop)`. In addition, the only types that should appear are "cell_t" and "float". No other type should be used for scalars. For arrays, "char" may be used, but not as a scalar. When using char arrays, the C++ definition must be careful to convert the char size to a cell-aligned size. For example: enum struct MyStruct { int x; int y; char message[50]; } Should look like this in C++: #pragma pack(push, 1) struct MyStruct { cell_t x; cell_t y; char message[CharArraySize<50>::bytes]; }; #pragma pack(pop) --- compiler/code-generator.cpp | 52 ++++++++++++++++++++++++-- compiler/code-generator.h | 1 + compiler/messages.h | 4 +- compiler/name-resolution.cpp | 6 --- compiler/semantics.cpp | 23 ++++++++++-- compiler/semantics.h | 1 + include/sp_typeutil.h | 6 +++ tests/compile-only/fail-newdecls-2.txt | 4 +- tests/enum-structs/native-return.out | 2 + tests/enum-structs/native-return.sp | 8 ++++ tests/shell.inc | 1 + third_party/amtl | 2 +- vm/shell/shell.cpp | 29 ++++++++++++++ 13 files changed, 122 insertions(+), 17 deletions(-) create mode 100644 tests/enum-structs/native-return.out create mode 100644 tests/enum-structs/native-return.sp diff --git a/compiler/code-generator.cpp b/compiler/code-generator.cpp index 5f08dd686..cf2683b38 100644 --- a/compiler/code-generator.cpp +++ b/compiler/code-generator.cpp @@ -1139,7 +1139,7 @@ CodeGenerator::EmitFieldAccessExpr(FieldAccessExpr* expr) void CodeGenerator::EmitCallExpr(CallExpr* call) { // If returning an array, push a hidden parameter. - if (call->fun()->return_array()) { + if (call->fun()->return_array() && !call->fun()->is_native()) { cell retsize = call->fun()->return_type()->CellStorageSize(); assert(retsize); @@ -1208,12 +1208,58 @@ void CodeGenerator::EmitCallExpr(CallExpr* call) { __ emit(OP_PUSH_PRI); } - EmitCall(call->fun(), (cell)argv.size()); + cell_t hidden_args = 0; + if (call->fun()->return_array() && call->fun()->is_native()) { + EmitNativeCallHiddenArg(call); + hidden_args++; + } + + EmitCall(call->fun(), (cell)argv.size() + hidden_args); - if (call->fun()->return_array()) + if (call->fun()->return_array() && !call->fun()->is_native()) __ emit(OP_POP_PRI); } +void CodeGenerator::EmitNativeCallHiddenArg(CallExpr* call) { + TrackTempHeapAlloc(call, 1); + + auto fun = call->fun(); + + ArrayData array; + BuildCompoundInitializer(fun->return_type(), nullptr, &array); + + cell retsize = call->fun()->return_type()->CellStorageSize(); + assert(retsize); + + __ emit(OP_HEAP, retsize * sizeof(cell)); + + auto info = fun->return_array(); + if (array.iv.empty()) { + __ emit(OP_CONST_PRI, 0); + __ emit(OP_FILL, retsize); + } else { + if (!info->iv_size) { + // No initializer, so we should have no data. + assert(array.data.empty()); + assert(array.zeroes); + + info->iv_size = (cell_t)array.iv.size(); + info->dat_addr = data_.dat_address(); + info->zeroes = array.zeroes; + data_.Add(std::move(array.iv)); + } + + cell dat_addr = info->dat_addr; + cell iv_size = info->iv_size; + assert(iv_size); + assert(info->zeroes); + + __ emit(OP_INITARRAY_ALT, dat_addr, iv_size, 0, info->zeroes, 0); + } + + __ emit(OP_PUSH_ALT); +} + void CodeGenerator::EmitDefaultArgExpr(DefaultArgExpr* expr) { diff --git a/compiler/code-generator.h b/compiler/code-generator.h index 479b4bca7..36670be12 100644 --- a/compiler/code-generator.h +++ b/compiler/code-generator.h @@ -90,6 +90,7 @@ class CodeGenerator final void EmitIndexExpr(IndexExpr* expr); void EmitFieldAccessExpr(FieldAccessExpr* expr); void EmitCallExpr(CallExpr* expr); + void EmitNativeCallHiddenArg(CallExpr* expr); void EmitDefaultArgExpr(DefaultArgExpr* expr); void EmitCallUserOpExpr(CallUserOpExpr* expr); void EmitNewArrayExpr(NewArrayExpr* expr); diff --git a/compiler/messages.h b/compiler/messages.h index e3600e8ab..6787e29ae 100644 --- a/compiler/messages.h +++ b/compiler/messages.h @@ -63,7 +63,7 @@ static const char* errmsg[] = { /*036*/ "empty statement\n", /*037*/ "invalid string (possibly non-terminated string)\n", /*038*/ "extra characters on line\n", - /*039*/ "unused39\n", + /*039*/ "natives can only return fixed-size arrays\n", /*040*/ "duplicate \"case\" label (value %d)\n", /*041*/ "invalid ellipsis, array size is not known\n", /*042*/ "invalid combination of class specifiers\n", @@ -165,7 +165,7 @@ static const char* errmsg[] = { /*138*/ "const was specified twice\n", /*139*/ "could not find type \"%s\"\n", /*140*/ "function '%s' does not return a value\n", - /*141*/ "natives, forwards, and public functions cannot return arrays\n", + /*141*/ "forwards and public functions cannot return arrays\n", /*142*/ "unexpected array expression\n", /*143*/ "new-style declarations should not have \"new\"\n", /*144*/ "void cannot be used as a variable type\n", diff --git a/compiler/name-resolution.cpp b/compiler/name-resolution.cpp index 222c5d96a..ce63100c1 100644 --- a/compiler/name-resolution.cpp +++ b/compiler/name-resolution.cpp @@ -790,12 +790,6 @@ bool FunctionDecl::Bind(SemaContext& outer_sc) { markusage(args_[0], uREAD); } - if ((is_native_ || is_public_ || is_forward_) && - (return_type()->isArray() || return_type()->isEnumStruct())) - { - error(pos_, 141); - } - ok &= BindArgs(sc); if (body_) diff --git a/compiler/semantics.cpp b/compiler/semantics.cpp index 732d79278..e48d6f486 100644 --- a/compiler/semantics.cpp +++ b/compiler/semantics.cpp @@ -2522,6 +2522,24 @@ bool Semantics::CheckCompoundReturnStmt(ReturnStmt* stmt) { return true; } +bool Semantics::CheckNativeCompoundReturn(FunctionDecl* info) { + auto rt = info->return_type(); + if (auto root = rt->as()) { + for (auto it = root; it; it = it->inner()->as()) { + if (it->size() == 0) { + report(info, 39); + return false; + } + } + } + + // For native calls, the implicit arg is first, not last. + auto rai = new FunctionDecl::ReturnArrayInfo; + rai->hidden_address = sizeof(cell_t) * 3; + info->set_return_array(rai); + return true; +} + bool Semantics::CheckAssertStmt(AssertStmt* stmt) { if (Expr* expr = AnalyzeForTest(stmt->expr())) { stmt->set_expr(expr); @@ -2857,10 +2875,9 @@ bool Semantics::CheckFunctionDeclImpl(FunctionDecl* info) { } if (info->is_native()) { - if (decl.type.dim_exprs.size() > 0) { - report(info->pos(), 83); + auto rt = info->return_type(); + if ((rt->isArray() || rt->isEnumStruct()) && !CheckNativeCompoundReturn(info)) return false; - } return true; } diff --git a/compiler/semantics.h b/compiler/semantics.h index d5518f100..4507be285 100644 --- a/compiler/semantics.h +++ b/compiler/semantics.h @@ -184,6 +184,7 @@ class Semantics final bool CheckStaticAssertStmt(StaticAssertStmt* stmt); bool CheckReturnStmt(ReturnStmt* stmt); bool CheckCompoundReturnStmt(ReturnStmt* stmt); + bool CheckNativeCompoundReturn(FunctionDecl* info); bool CheckExprStmt(ExprStmt* stmt); bool CheckIfStmt(IfStmt* stmt); bool CheckConstDecl(ConstDecl* decl); diff --git a/include/sp_typeutil.h b/include/sp_typeutil.h index 3812539cf..5864ffb74 100644 --- a/include/sp_typeutil.h +++ b/include/sp_typeutil.h @@ -77,4 +77,10 @@ sp_ctof(cell_t val) return sp::FloatCellUnion(val).f32; } +template +struct CharArraySize { + static constexpr size_t cells = (Size + sizeof(cell_t) - 1) / sizeof(cell_t); + static constexpr size_t bytes = cells * sizeof(cell_t); +}; + #endif //_INCLUDE_SOURCEPAWN_VM_TYPEUTIL_H_ diff --git a/tests/compile-only/fail-newdecls-2.txt b/tests/compile-only/fail-newdecls-2.txt index 8b406f063..e9696bad8 100644 --- a/tests/compile-only/fail-newdecls-2.txt +++ b/tests/compile-only/fail-newdecls-2.txt @@ -1,2 +1,2 @@ -(1) : error 141: natives, forwards, and public functions cannot return arrays -(2) : error 141: natives, forwards, and public functions cannot return arrays +(1) : error 039: natives can only return fixed-size arrays +(2) : error 141: forwards and public functions cannot return arrays diff --git a/tests/enum-structs/native-return.out b/tests/enum-structs/native-return.out new file mode 100644 index 000000000..18f942d86 --- /dev/null +++ b/tests/enum-structs/native-return.out @@ -0,0 +1,2 @@ +x: 4 +y: 6 diff --git a/tests/enum-structs/native-return.sp b/tests/enum-structs/native-return.sp new file mode 100644 index 000000000..53222605a --- /dev/null +++ b/tests/enum-structs/native-return.sp @@ -0,0 +1,8 @@ +#include + +public main() +{ + TestStruct a = {1, 2}; + TestStruct b = {3, 4}; + print_test_struct(add_test_structs(a, b)); +} diff --git a/tests/shell.inc b/tests/shell.inc index ddb4b3207..593984658 100644 --- a/tests/shell.inc +++ b/tests/shell.inc @@ -52,3 +52,4 @@ native void copy_2d_array_to_callback(const int[] flat_array, int length, int st native void assert_eq(any a1, any a2); native void print_test_struct(TestStruct x); +native TestStruct add_test_structs(TestStruct a, TestStruct b); diff --git a/third_party/amtl b/third_party/amtl index e38cfe3cb..2d3b1a337 160000 --- a/third_party/amtl +++ b/third_party/amtl @@ -1 +1 @@ -Subproject commit e38cfe3cbf2047916e4a68017840bd325a8f7080 +Subproject commit 2d3b1a3378a3728637f26660c9ffc2df3189cf62 diff --git a/vm/shell/shell.cpp b/vm/shell/shell.cpp index d7ab6ad14..fce0e30b0 100644 --- a/vm/shell/shell.cpp +++ b/vm/shell/shell.cpp @@ -327,6 +327,24 @@ static cell_t PrintTestStruct(IPluginContext* cx, const cell_t* params) { return 0; } +static cell_t AddTestStructs(IPluginContext* cx, const cell_t* params) { + TestStruct* a; + TestStruct* b; + TestStruct* out; + + int err; + if ((err = cx->LocalToPhysAddr(params[1], reinterpret_cast(&out))) != SP_ERROR_NONE) + return cx->ThrowNativeErrorEx(err, "Could not read out argument"); + if ((err = cx->LocalToPhysAddr(params[2], reinterpret_cast(&a))) != SP_ERROR_NONE) + return cx->ThrowNativeErrorEx(err, "Could not read argument 1"); + if ((err = cx->LocalToPhysAddr(params[3], reinterpret_cast(&b))) != SP_ERROR_NONE) + return cx->ThrowNativeErrorEx(err, "Could not read argument 2"); + + out->x = a->x + b->x; + out->y = a->y + b->y; + return params[1]; +} + class DynamicNative : public INativeCallback { public: @@ -353,6 +371,16 @@ class DynamicNative : public INativeCallback uintptr_t refcount_ = 0; }; +#pragma pack(push, 1) +struct LayoutVerifier { + char message[CharArraySize<50>::bytes]; + int x; +}; +#pragma pack(pop) + +static_assert(offsetof(LayoutVerifier, message) == 0); +static_assert(offsetof(LayoutVerifier, x) == 52); + static int Execute(const char* file) { char error[255]; @@ -386,6 +414,7 @@ static int Execute(const char* file) BindNative(rt, "assert_eq", AssertEq); BindNative(rt, "printf", Printf); BindNative(rt, "print_test_struct", PrintTestStruct); + BindNative(rt, "add_test_structs", AddTestStructs); IPluginFunction* fun = rt->GetFunctionByName("main"); if (!fun)