Forward arguments in ResourceLimitedVector::emplace_back

[MaxFreedomPollard]

Description

ResourceLimitedVector::emplace_back takes a forwarding reference pack and its Doxygen block documents it as "Arguments forwarded to construct the new element", but include/fastdds/utils/collections/ResourceLimitedVector.hpp:227 expands that pack as plain lvalues: collection_.emplace_back(args ...);. The value category of every argument is dropped, so an rvalue argument is copied instead of moved, and a move-only element type does not compile at all because the underlying call is instantiated as emplace_back<std::unique_ptr<int> &>. The sibling push_back(value_type&&) a few lines above does collection_.push_back(std::move(val)), so the two entry points behave differently on identical input.

The fix is to expand the pack as collection_.emplace_back(std::forward<Args>(args)...) and add #include <utility> for it. The signature, the Doxygen block and the mangled names are unchanged, so this only makes the code do what the documentation already said.

No call site inside the repository changes behaviour. The only emplace_back(std::move(...)) calls under src/ are on std::vector members (NetworkFactory::mRegisteredTransports and the consumers vector in SharedMemLog.hpp), not on a ResourceLimitedVector.

Two regression tests are added to test/unittest/utils/ResourceLimitedVectorTests.cpp: emplace_back_forwards_arguments counts constructions and asserts an lvalue is copied once and an rvalue is moved once, and emplace_back_move_only_value emplaces a std::unique_ptr<int> and checks the source is left empty.

Verification

Apple clang 17 on macOS, building ResourceLimitedVectorTests.cpp standalone against the unmodified header and against the patched one.

Against the unmodified header from 0f5c9b9 the file does not compile, and the diagnostic points straight at line 227:

$ clang++ -std=c++17 -I baseline-include -I googletest/include \
    test/unittest/utils/ResourceLimitedVectorTests.cpp libgtest.a -pthread -o rlv_base
error: call to implicitly-deleted copy constructor of 'std::unique_ptr<int>'
ResourceLimitedVector.hpp:227:21: note: in instantiation of function template specialization
  'std::vector<std::unique_ptr<int>>::emplace_back<std::unique_ptr<int> &>' requested here
  227 |         collection_.emplace_back(args ...);

Removing only emplace_back_move_only_value so the rest builds against the unmodified header, the other new test fails and the five pre-existing tests pass:

[ RUN      ] ResourceLimitedVectorTests.emplace_back_forwards_arguments
Failure: Expected equality of these values:
  moved->copies
    Which is: 1
  0u
    Which is: 0
[  FAILED  ] ResourceLimitedVectorTests.emplace_back_forwards_arguments
[  PASSED  ] 5 tests.

Against the patched header all seven tests pass, with -Wall -Wextra and no warnings:

$ clang++ -std=c++17 -Wall -Wextra -I include -I googletest/include \
    test/unittest/utils/ResourceLimitedVectorTests.cpp libgtest.a -pthread -o rlv_fixed
[==========] 7 tests from 1 test suite ran. (0 ms total)
[  PASSED  ] 7 tests.

A small probe counting copy and move constructions shows the difference directly. Unmodified header: emplace_back(std::move(t)) -> copies=1 moves=0 and push_back(std::move(t)) -> copies=0 moves=1. Patched header: emplace_back(std::move(t)) -> copies=0 moves=1 and push_back(std::move(t)) -> copies=0 moves=1.

Both changed files are left byte for byte unchanged by uncrustify 0.78.1 with the project configuration.

Backports

@Mergifyio backport 3.2.x

The same expansion is present on 3.6.x, 3.2.x, 2.14.x and 2.6.x. On 2.14.x and 2.6.x the header lives at include/fastrtps/utils/collections/ResourceLimitedVector.hpp, so those branches need a manual port rather than a clean cherry-pick. Happy to open them if you want the fix there.

Contributor Checklist

• Commit messages follow the project guidelines.
• The code follows the style guidelines of this project.
• Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
• N/A: Any new/modified methods have been properly documented using Doxygen.
• N/A: Any new configuration API has an equivalent XML API (with the corresponding XSD extension)
• Changes are backport compatible: they do NOT break ABI nor change library core behavior.
• Changes are API compatible.
• N/A: New feature has been added to the versions.md file (if applicable).
• N/A: New feature has been documented/Current behavior is correctly described in the documentation.
• Applicable backports have been included in the description.

Reviewer Checklist

• The PR has a milestone assigned.
• The title and description correctly express the PR's purpose.
• Check contributor checklist is correct.
• If this is a critical bug fix, backports to the critical-only supported branches have been requested.
• Check CI results: changes do not issue any warning.
• Check CI results: failing tests are unrelated with the changes.