Skip to content

[CUS-538] - $fork scope union struct support#167

Merged
akashlevy merged 13 commits into
mainfrom
union-struct
May 27, 2026
Merged

[CUS-538] - $fork scope union struct support#167
akashlevy merged 13 commits into
mainfrom
union-struct

Conversation

@stanminlee
Copy link
Copy Markdown

Support for union struct declared as $fork scopes.

  1. Create helper function that populates data structure with variable name (useful for preventing duplicate logic)
  2. Logic for union structs
  • when scoping, we track if we have entered a fork scope
  • if within a fork scope and we encounter a variable, we populate a vector with variables, otherwise, use helper to emit as normal
  • once we upscope, if we are coming from a fork scope, we check if it is a union (>= 2 vars all with same fstId). If it is a union, we treat the entire scope as a variable and emit it using helper
  • if it is not a fork scope, we loop through all variables in the vector and emit using helper function

@linear
Copy link
Copy Markdown

linear Bot commented May 26, 2026

CUS-538

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented May 26, 2026

Greptile Summary

This PR refactors extractVarNames to pull variable-registration logic into a new registerVar helper, and adds detection of union structs that are declared as $fork scopes in FST — treating the entire fork as a single variable when all its members share the same fstHandle.

  • registerVar helper: consolidates vars, handle_to_var, name_to_handle, and memory_to_handle population into one reusable function, removing duplicated logic from extractVarNames.
  • Union detection: on every FST_HT_UPSCOPE while in_fork is set, the code checks whether all collected variables share the same handle; if so it registers the fork name as a single variable at the parent scope, otherwise it registers each variable individually.

Confidence Score: 3/5

The change works correctly for flat fork scopes with direct variables, but will silently misregister variables for any union whose members are themselves structs with nested scopes.

The in_fork flag is cleared on the first FST_HT_UPSCOPE seen inside the fork, regardless of nesting depth. For a union-of-structs FST layout this causes union-detection to fire prematurely, variables to be registered under the wrong scope, and the outer union to be completely missed — all without any warning or error.

kernel/fstdata.cc — specifically the FST_HT_UPSCOPE branch in extractVarNames and the depth-tracking logic around in_fork.

Important Files Changed

Filename Overview
kernel/fstdata.cc Extracts variable-registration logic into a new registerVar helper and adds $fork-scope union detection in extractVarNames. The union detection has a depth-tracking bug that causes premature processing on nested sub-scopes within a fork.
kernel/fstdata.h Adds the registerVar private method declaration. No issues found.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[fstReaderIterateHier loop] --> B{h->htyp}
    B -->|FST_HT_SCOPE| C{in_fork == false AND typ == FST_ST_VCD_FORK?}
    C -->|Yes| D[Set in_fork=true / Save fork_parent_scope and fork_name / Clear fork_vars]
    C -->|No| E[fstReaderPushScope]
    D --> E
    B -->|FST_HT_UPSCOPE| F{in_fork?}
    F -->|Yes| G{fork_vars.size >= 2 AND all ids equal?}
    G -->|Yes = union| H[Create single FstVar with fork_name and fork_parent_scope / registerVar]
    G -->|No = not union| I[Loop: registerVar for each fork_var]
    H --> J[in_fork=false / fork_vars.clear]
    I --> J
    J --> K[fstReaderPopScope]
    F -->|No| K
    B -->|FST_HT_VAR| L{in_fork?}
    L -->|Yes| M[fork_vars.push_back var]
    L -->|No| N[registerVar var]
Loading

Reviews (1): Last reviewed commit: "union" | Re-trigger Greptile

Comment thread kernel/fstdata.cc
Comment thread kernel/fstdata.cc
Comment thread kernel/fstdata.cc
Copy link
Copy Markdown

@akashlevy akashlevy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See this wasn't that bad haha

stanminlee and others added 6 commits May 26, 2026 22:47
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Removed unnecessary closing brace in switch case.
Removed logic for nested $fork handling in union struct detection.
@akashlevy akashlevy merged commit a1fa854 into main May 27, 2026
8 checks passed
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

Successfully merging this pull request may close these issues.

2 participants