From eb02dbe2b2143c0f2092afe83a4121f1ac342fcc Mon Sep 17 00:00:00 2001 From: Tim Snyder Date: Mon, 2 Dec 2024 19:07:14 -0500 Subject: [PATCH 1/2] Why does `collect()` silently ignore unexpected positional args? Opening a PR for this because it is the easiest way to see if the added test still fails with latest sparta without creating an updated env locally. I would expect that `pevent.collect()` should throw if called positional arguments that are: * unexpected (i.e. the pevent wasn't defined to take any positional args) * mismatching type (as it looks like compilation doesn't catch this) - didn't commit the test I added for this in this commit * what other sanity checking is feasible? The test added in this PR shows that we can call `collect()` with positional args for events that haven't registered any positional args. Why? As it is, code in the wild has become quite confusing where positional args are being passed to `collect()` and silently ignored. I'm looking at how to propose improving this part of sparta but I wanted to start a discussion but first create a PR that will demonstrate that this behavior isn't considered a throwable conidtion in latest sparta. --- sparta/test/PEvents/PEventHelper_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/sparta/test/PEvents/PEventHelper_test.cpp b/sparta/test/PEvents/PEventHelper_test.cpp index 549cc70c6d..f09747c363 100644 --- a/sparta/test/PEvents/PEventHelper_test.cpp +++ b/sparta/test/PEvents/PEventHelper_test.cpp @@ -97,6 +97,7 @@ int main() { trigger.go(); A a(1000, 78, 52, 0.01, "test0"); pair_pevent.collect(a); + EXPECT_THROW(pair_event.collect(a, "unexpected positional pair arg")); pair_verbose_pevent.collect(a); decode_pevent.collect(a); my_pevent.collect(a, 32); From 21a43b3971cb0e99b7883ca959306cc6adad7518 Mon Sep 17 00:00:00 2001 From: Tim Snyder Date: Mon, 2 Dec 2024 21:52:14 -0500 Subject: [PATCH 2/2] fix typo --- sparta/test/PEvents/PEventHelper_test.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sparta/test/PEvents/PEventHelper_test.cpp b/sparta/test/PEvents/PEventHelper_test.cpp index f09747c363..31899d80d5 100644 --- a/sparta/test/PEvents/PEventHelper_test.cpp +++ b/sparta/test/PEvents/PEventHelper_test.cpp @@ -97,7 +97,7 @@ int main() { trigger.go(); A a(1000, 78, 52, 0.01, "test0"); pair_pevent.collect(a); - EXPECT_THROW(pair_event.collect(a, "unexpected positional pair arg")); + EXPECT_THROW(pair_pevent.collect(a, "unexpected positional pair arg")); pair_verbose_pevent.collect(a); decode_pevent.collect(a); my_pevent.collect(a, 32);