Skip to content

Conversation

lucioleKi
Copy link
Contributor

Fix #10119. This fix works for 26 and 27.

@lucioleKi lucioleKi self-assigned this Aug 20, 2025
@lucioleKi lucioleKi added team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI labels Aug 20, 2025
Copy link
Contributor

github-actions bot commented Aug 20, 2025

CT Test Results

  2 files   13 suites   4m 26s ⏱️
112 tests 110 ✅ 2 💤 0 ❌
128 runs  126 ✅ 2 💤 0 ❌

Results for commit 0963ee5.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@lucioleKi
Copy link
Contributor Author

Map-generator handling in 28 is fixed by 0f0e375.
Yet we need to add extra code to handle strict generators and zip
generators. I'll make a separate PR for 28.

@lucioleKi lucioleKi requested a review from bjorng August 20, 2025 13:12
Comment on lines +760 to +780
vann_map_comp_body_join() ->
fun (T, {Env, Bound, Free}) ->
{T1, Bound1, Free1} = case erl_syntax:type(T) of
binary_generator ->
vann_binary_generator(T,Env);
generator ->
vann_generator(T, Env);
map_generator ->
vann_map_generator(T,Env);
_ ->
%% Bindings in filters are not
%% exported to the rest of the
%% body.
{T2, _, Free2} = vann(T, Env),
{T2, [], Free2}
end,
Env1 = ordsets:union(Env, Bound1),
{T1, {Env1, ordsets:union(Bound, Bound1),
ordsets:union(Free,
ordsets:subtract(Free1, Bound))}}
end.
Copy link
Contributor

Choose a reason for hiding this comment

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

Stylish question, there are now three (3) copies of this function (vann_map_comp_body_join, vann_list_comp_body_join and vann_binary_comp_body_join where the last one has a slightly different indentation!) they include nothing map/list/binary specific, could they be replaced by a single vann_comp_body_join? That should make future updates less error prone? 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants