Skip to content

src: replace push_back(T{}) with emplace_back() #65227

Merged
idryomov merged 2 commits intoceph:mainfrom
tchaikov:wip-generate-brace-init
Sep 16, 2025
Merged

src: replace push_back(T{}) with emplace_back() #65227
idryomov merged 2 commits intoceph:mainfrom
tchaikov:wip-generate-brace-init

Conversation

@tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Aug 26, 2025

Contribution Guidelines

  • To sign and title your commits, please refer to Submitting Patches to Ceph.

  • If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.

  • When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands

in ed6b712, we switched from some instances of `push_back(Type{})`
patterns to `emplace_back()` to simplify the code and to avoid
unnecessary overhead of copy/move constructor calls. but there were
still remaining instances of them in `generate_test_instance()`
member function implementations.

in this change, we switched all of them to be more consistent.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
…ances()

in ed6b712, we switched some of them to explicit call of constructor to
study some compiling failures, but forgot to revert to the original
version after the failures were addressed.

in this change, we revert them to the original version to be more consistent.

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov tchaikov requested review from a team as code owners August 26, 2025 05:41
@tchaikov tchaikov changed the title Wip generate brace init src: replace push_back(T{}) with emplace_back() Aug 26, 2025
@tchaikov tchaikov requested a review from idryomov August 26, 2025 05:42
@tchaikov
Copy link
Contributor Author

jenkins test make check arm64

1 similar comment
@tchaikov
Copy link
Contributor Author

jenkins test make check arm64

Copy link
Contributor

@ronen-fr ronen-fr left a comment

Choose a reason for hiding this comment

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

LGTM
(note: additional teams involved)

Copy link
Contributor

@idryomov idryomov left a comment

Choose a reason for hiding this comment

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

Ack from the RBD perspective

@anrao19
Copy link
Contributor

anrao19 commented Sep 16, 2025

pr testing of RGW completed : https://tracker.ceph.com/issues/73015 and got approved by @ivancich

@idryomov idryomov merged commit 28ea896 into ceph:main Sep 16, 2025
29 checks passed
@tchaikov tchaikov deleted the wip-generate-brace-init branch September 16, 2025 07:33
@ljflores
Copy link
Member

@tchaikov @idryomov it seems like this PR should have also gone through the rados and cephfs suites- was there a reason why this didn't happen?

@tchaikov
Copy link
Contributor Author

@ljflores hi Laura, thank you for the reminder. But the change only touches the code used for generating the corpus for performing encoding tests. These corpus data are only used in our CI pipeline. IIUC, our rados tests performed by teuthology does not exercise or use these corpus.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants