Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arrays of typedef struct X { content } X_t produce "TypeError ... object does not support" errors, is this expected? #1832

Open
ddj116 opened this issue Jan 15, 2025 · 2 comments
Assignees

Comments

@ddj116
Copy link
Contributor

ddj116 commented Jan 15, 2025

The issue

Somewhere between Trick 19.7.1 and 19.7.3 the Ramtares team has witnessed a change in behavior when assigning variables in the input file under an array of C-structs defined by the standard we have adopted in our coding standards, and demonstrated on the SIM_ball_L1 example sim below with this diff patch:

diff --git a/trick_sims/Ball/SIM_ball_L1/RUN_test/input.py b/trick_sims/Ball/SIM_ball_L1/RUN_test/input.py
index 461cbce4..c2e4860f 100644
--- a/trick_sims/Ball/SIM_ball_L1/RUN_test/input.py
+++ b/trick_sims/Ball/SIM_ball_L1/RUN_test/input.py
@@ -15,3 +15,5 @@ trick.add_read(read, "trick.ball_print(ball.state)")
 # Set the stop time
 trick.exec_set_terminate_time(300.0)
 
+ball.fake[0] = 0           # produces TypeError: 'FAKE_t' object does not support item assignment
+#ball.fake[0].real.bar = 1 # produces TypeError: 'FAKE_t' object does not support indexing
diff --git a/trick_sims/Ball/SIM_ball_L1/S_define b/trick_sims/Ball/SIM_ball_L1/S_define
index 88698bd9..ab3ba23c 100644
--- a/trick_sims/Ball/SIM_ball_L1/S_define
+++ b/trick_sims/Ball/SIM_ball_L1/S_define
@@ -81,6 +81,7 @@ class ballSimObject : public Trick::SimObject {
          */
         BSTATE      state ;
         BFORCE      force ;
+        FAKE        fake[2];
 
         /*
          * Jobs are declared in the constructors of the SimObject.  The job syntax is
diff --git a/trick_sims/Ball/models/ball/L1/include/ball_state.h b/trick_sims/Ball/models/ball/L1/include/ball_state.h
index bfb3e4ed..d3dbba54 100644
--- a/trick_sims/Ball/models/ball/L1/include/ball_state.h
+++ b/trick_sims/Ball/models/ball/L1/include/ball_state.h
@@ -62,4 +62,14 @@ typedef struct { /* BSTATE ---------------------------------------------------*/
 
 } BSTATE ; /*-----------------------------------------------------------------*/
 
+typedef struct REAL {
+  int bar;
+} REAL_t ;
+
+typedef struct FAKE {
+  int foo;
+  REAL_t real;
+} FAKE_t ;
+
+
 #endif

With these changes applied to the origin/master branch, execution of RUN_test/input.py produces errors that look like this:

Traceback (most recent call last):
  File "RUN_test/input.py", line 18, in <module>
    ball.fake[0] = 0
TypeError: 'FAKE_t' object does not support item assignment

Workarounds

In a debug session in mid December with @sharmeye and @keithvetter, we speculated this to be downstream of the changes to convert_swig and found that a workaround was to make the struct definitions anonymous. For example, changing

typedef struct FAKE {
  int foo;
  REAL_t real;
} FAKE_t ;

to

typedef struct {
  int foo;
  REAL_t real;
} FAKE_t ;

...and using the FAKE_t name for all references to the struct. Other workarounds that are viable, as copied from our internal thread:

  • use the typedef'd name as the C-struct name typedef struct XXX{...} XXX;. In this case, probably just drop the _t suffix entirely.
  • separate out the definition and the typedef into 2 statements: struct XXX {...}; typedef struct XXX XXX_t;

Understanding the right path forward

The state represented in the diff above no longer works, so ultimately what we're after is understanding if this change in behavior was deliberate and Trick is now acting as it should, or if this is unintended and will be treated as a bug and fixed in a later Trick release. We can adjust our code to use one of the workarounds but before we go down that path we want to know if it's the right path forward.


FYI @Pherring04 @hchen99 @jmpenn for awareness, thanks!

@hchen99
Copy link
Contributor

hchen99 commented Jan 16, 2025

Thank you for providing all the details. We'll look into this.

@sharmeye sharmeye assigned sharmeye and Pherring04 and unassigned sharmeye Jan 28, 2025
@ninotarantino
Copy link
Contributor

I caught this a while back in #1351, looks like the same issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants