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.

Sign up for GitHub

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

Jump to bottom

Initialize first field in named union constructors #784

Merged
merged 2 commits into from
Jun 16, 2024
Merged

Initialize first field in named union constructors #784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dd88a9b
Revert "denormalize specdata union into inline unions so it can be in…
lethosor Jun 15, 2024
17e4c6d
Initialize first field in named union constructors
lethosor Jun 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion StructFields.pm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,13 @@ sub emit_struct_fields($$;%) {
} 'fields-' . $fields_group;

# Needed for unions with fields with non-default ctors (e.g. bitfields)
emit "$name(){memset(this, 0, sizeof($name));}";
emit_block {
local $in_union = 1;
Copy link
Member

Choose a reason for hiding this comment

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

is this variable unused?

Copy link
Member

@ab9rf ab9rf Jun 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

No, it's used to tell render_field_init that it's rendering fields from a union, and is necessary here to get the behavior we want.

Remember that in Perl, local variables are dynamically, not lexically, scoped (see What is the difference between my and local in Perl?).

Copy link
Member Author

@lethosor lethosor Jun 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

It's read here in render_field_init but is not set there:

df-structures/StructFields.pm

Line 409 in 833a1fd

local $in_union = $in_union || $is_union;

Probably an oversimplification: I've been thinking of local vars as additional context args that get pushed on the stack and get restored when they go "out of scope". Inspecting $is_union outside of this emit_block will show it still set to 0.

Copy link
Member

Choose a reason for hiding this comment

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

they're actually pushed into the current block's dynamics symbol table, which remains visible during a subroutine call, so any subroutine called will also see them (unless shadowed). lexically scoped my locals, however, are only visible within the lexical block (and any enclosed blocks) in which they're created.

it's not so much that they get "restored" but rather that defining a symbol within a block shadows any definition that might exist in a block further up. when the block ends, its symbol tables are disposed (each block effectively has two, one for dynamic scope and one for lexical scope), and any prior definition that was being shadowed is thereby unshadowed and thus once again visible. every block carries with it a pointer to the lexical context in which it was defined (a closure), and the current dynamic symbol context is used to initialize a new dynamic symbol context whenever execution enters a new block.

Copy link
Member

Choose a reason for hiding this comment

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

thank for the explanation. my perl fu is low

emit "memset(this, 0, sizeof(*this));";
# for unions with fields defined, initialize the first one
my $first_child = ($tag->findnodes('ld:field[1]'))[0];
render_field_init($first_child);
} "$name() ";

return;
}
Expand Down
24 changes: 9 additions & 15 deletions df.jobs.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,12 @@
<bitfield name='encrust_flags' type-name='stockpile_group_set'/>
</struct-type>

<struct-type type-name='job_spec_data' is-union='true'>
<int32_t name="hist_figure_id" ref-target='historical_figure'/>
<int32_t name="race" ref-target='creature_raw'/>
<enum base-type='int32_t' name="improvement" type-name='improvement_type'/>
</struct-type>

<struct-type type-name='job' original-name='jobst' df-list-link-type='job_list_link' df-list-link-field='list_link' key-field='id'>
<int32_t name='id' init-value='-1'/>
<pointer name='list_link' type-name='job_list_link'/>
Expand All @@ -150,11 +156,7 @@
<int16_t name='item_subtype' init-value='-1' comment='when StoreInStockpile this is a unit_labor'/>

<compound name='specflag' type-name='job_spec_flags'/>
<compound name='specdata' is-union='true' init-value='-1'>
<int32_t name="hist_figure_id" ref-target='historical_figure'/>
<int32_t name="race" ref-target='creature_raw'/>
<enum base-type='int32_t' name="improvement" type-name='improvement_type'/>
</compound>
<compound name='specdata' type-name='job_spec_data'/>
<bitfield name='material_category' type-name='job_material_category' comment='bay12: uint32_t job_item_flag'/>

<stl-string name='reaction_name'/>
Expand Down Expand Up @@ -449,11 +451,7 @@
<int32_t name="mat_index" init-value='-1'/>

<compound name='specflag' type-name='job_spec_flags'/>
<compound name='specdata' is-union='true' init-value='-1'>
<int32_t name="hist_figure_id" ref-target='historical_figure'/>
<int32_t name="race" ref-target='creature_raw'/>
<enum base-type='int32_t' name="improvement" type-name='improvement_type'/>
</compound>
<compound name='specdata' type-name='job_spec_data'/>
<bitfield name="material_category" type-name='job_material_category'/>

<compound name='art_spec' type-name='job_art_specification'/>
Expand Down Expand Up @@ -524,11 +522,7 @@
<int16_t name="mat_type" ref-target='material' aux-value='$$.mat_index'/>
<int32_t name="mat_index"/>
<compound name='specflag' type-name='job_spec_flags'/>
<compound name='specdata' is-union='true' init-value='-1'>
<int32_t name="hist_figure_id" ref-target='historical_figure'/>
<int32_t name="race" ref-target='creature_raw'/>
<enum base-type='int32_t' name="improvement" type-name='improvement_type'/>
</compound>
<compound name='specdata' type-name='job_spec_data'/>
<bitfield name="material_category" type-name='job_material_category'/>
<int32_t name='match_value' init-value='1'/>

Expand Down
6 changes: 1 addition & 5 deletions df.ui-menus.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -263,11 +263,7 @@
<int16_t name='material' ref-target='material' aux-value='$$.matgloss'/>
<int32_t name='matgloss'/>
<compound name='specflag' type-name='job_spec_flags'/>
<compound name='specdata' is-union='true' init-value='-1'>
<int32_t name="hist_figure_id" ref-target='historical_figure'/>
<int32_t name="race" ref-target='creature_raw'/>
<enum base-type='int32_t' name="improvement" type-name='improvement_type'/>
</compound>
<compound name='specdata' type-name='job_spec_data'/>
<bitfield name='job_item_flag' type-name='job_material_category'/>
<bool name='add_building_location'/>
<bool name='show_help_instead'/>
Expand Down