Skip to content

SMV: avoid some expression copies #999

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

Merged
merged 1 commit into from
Feb 19, 2025
Merged

SMV: avoid some expression copies #999

merged 1 commit into from
Feb 19, 2025

Conversation

kroening
Copy link
Member

References are changed to rvalue references to avoid some copying when populating the parse tree.

@kroening kroening added the SMV label Feb 18, 2025
@kroening kroening force-pushed the smv-parse-tree-move branch from 7a44cc3 to 97b2b2b Compare February 18, 2025 20:59
@kroening kroening marked this pull request as ready for review February 19, 2025 09:45
Comment on lines 491 to 494
exprt &op=stack_expr($3);
unary_exprt tmp(ID_smv_next, std::move(op));
tmp.swap(op);
PARSER.module->add_assign_next(to_equal_expr(stack_expr($$)));
PARSER.module->add_assign_next(std::move(stack_expr($3)), std::move(stack_expr($6)));
Copy link
Collaborator

Choose a reason for hiding this comment

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

How about:

unary_exprt tmp{ID_smv_next, stack_expr($3)};
PARSER.module->add_assign_next(std::move(tmp), std::move(stack_expr($6)));

even if just a human not having to track what really lives at $3 at which point in this sequence.

Comment on lines 124 to 127
items.push_back(itemt{});
items.back().item_type=item_type;
items.back().expr=expr;
items.back().location=location;
items.back().expr = std::move(expr);
items.back().location = std::move(location);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps itemt should have a proper constructor, at which point it should be possible to use
items.emplace_back(item_type, std::move(expr), std::move(location));. (Which in turns makes me wonder whether we could then just get rid of add_item as a wrapper altogether.)

References are changed to rvalue references to avoid some copying when
populating the parse tree.
@kroening kroening force-pushed the smv-parse-tree-move branch from 97b2b2b to a06ddb2 Compare February 19, 2025 11:39
@kroening kroening merged commit c7a04c0 into main Feb 19, 2025
9 checks passed
@kroening kroening deleted the smv-parse-tree-move branch February 19, 2025 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants