act icon indicating copy to clipboard operation
act copied to clipboard

refactor: Move Time t from TEntry to TItem for improved state handling

Open Stereco-btc opened this issue 8 months ago • 4 comments

This PR moves the timing information from TEntry to TItem to improve how we handle state references in the codebase.

Changes

  • Moved Time t parameter from TEntry to TItem constructor
  • Updated TEntry constructor to remove redundant timing parameter
  • Added eqItemKind helper function to properly handle type-safe comparisons of TItems with different RefKinds
  • Updated JSON serialization to include timing and reference kind information
  • Modified location extraction and name generation code to handle pre/post state versions

Technical Details

  • Changed TEntry from TEntry :: Pn -> Time t -> SRefKind k -> TItem a k t -> Exp a t to TEntry :: Pn -> SRefKind k -> TItem a k t -> Exp a t
  • Updated TItem to include timing: Item :: SType a -> ValueType -> Time t -> Ref k t -> TItem a k t
  • Added specialized eqItemKind function to fix type-checking issues with different RefKind parameters

Testing

The changes maintain all existing functionality while improving type safety and state handling. All existing tests should continue to pass.

Fixes #121

Stereco-btc avatar May 11 '25 04:05 Stereco-btc

Hey @kjekac, I’ve sent the PR, could you please review the changes when you get a chance? Just a heads-up, some CI checks are currently failing. Let me know if you'd like me to look into it.

Thanks!

Stereco-btc avatar May 17 '25 17:05 Stereco-btc

Hey @Stereco-btc, thanks for the contribution!

The CI is currently failing because the change needs to be propagated to all files that use these definitions.

That said, this is quite an old issue, and Act has undergone a major redesign since then. I haven't yet considered whether this refactoring is still desirable in the current context.

zoep avatar May 18 '25 15:05 zoep

Hi @zoep,

Thanks for the review and for clarifying the reason for the CI failures! I understand that the changes to TEntry and TItem need to be propagated throughout the codebase.

Regarding your comment about this being an old issue and Act having undergone a major redesign since then: before I proceed with propagating these changes, could you please let me know if this refactoring is still considered desirable in the current context of the project?

Thanks!

Stereco-btc avatar May 26 '25 14:05 Stereco-btc