Skip to content

Ensure option names updated to match path - #3445

Merged
ZedThree merged 2 commits into
nextfrom
cmacmackin/options-names
Jul 29, 2026
Merged

Ensure option names updated to match path#3445
ZedThree merged 2 commits into
nextfrom
cmacmackin/options-names

Conversation

@cmacmackin

Copy link
Copy Markdown
Collaborator

Previously, using the move-assignment operator for Options objects would keep the same full name as the moved object. This was problematic because it meant that the name of the newly-assigned Options object could differ from the path used to access that object. E.g.,

Options opts;
// Add various things in options
opts["a"]["b"] = opts["c"]["d"].copy();
fmt::print("{}\n", opts["a"]["b"].str()); // Output: c:d

This posed particular problems for the permissions system in Hermes-3, as it meant that the name being checked against permissions could be incorrect.

In this PR I've modified the move-assignment operator to ensure that the assigned-to Options object (and all children) will have names consistent with the path to access them.

@cmacmackin
cmacmackin requested a review from ZedThree July 28, 2026 16:47

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

Comment thread include/bout/options.hxx
@ZedThree
ZedThree merged commit 93d6705 into next Jul 29, 2026
29 of 30 checks passed
@ZedThree
ZedThree deleted the cmacmackin/options-names branch July 29, 2026 09:14
@cmacmackin

Copy link
Copy Markdown
Collaborator Author

Thanks for the speedy turnaround on this, but unfortunately I just realised there is a corner-case I missed. I'll open another PR for that.

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