Skip to content

Commit

Permalink
Merge pull request mockingbirdnest#3897 from pleroy/ReturnRef
Browse files Browse the repository at this point in the history 
Fix possible dangling references caused by ReturnRef
  • Loading branch information
@pleroy
pleroy authored Mar 14, 2024
2 parents 9382a8e + c53ced0 commit 4f09547
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 75 deletions.
101 changes: 53 additions & 48 deletions ksp_plugin_test/interface_flight_plan_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,31 @@ MATCHER(IsOk,
class InterfaceFlightPlanTest : public ::testing::Test {
protected:
InterfaceFlightPlanTest()
: plugin_(make_not_null_unique<StrictMock<MockPlugin>>()),
const_plugin_(plugin_.get()) {}

: adaptive_step_parameters_(
EmbeddedExplicitRungeKuttaNyströmIntegrator<
DormandالمكاوىPrince1986RKN434FM,
Ephemeris<Barycentric>::NewtonianMotionEquation>(),
/*max_steps=*/111,
/*length_integration_tolerance=*/222 * Metre,
/*speed_integration_tolerance=*/333 * Metre / Second),
generalized_adaptive_step_parameters_(
EmbeddedExplicitGeneralizedRungeKuttaNyströmIntegrator<
Fine1987RKNG34,
Ephemeris<Barycentric>::GeneralizedNewtonianMotionEquation>(),
/*max_steps=*/111,
/*length_integration_tolerance=*/222 * Metre,
/*speed_integration_tolerance=*/333 * Metre / Second),
identity_(Rotation<Barycentric, AliceSun>::Identity()),
plugin_(make_not_null_unique<StrictMock<MockPlugin>>()),
const_plugin_(plugin_.get()) {}

Ephemeris<Barycentric>::AdaptiveStepParameters adaptive_step_parameters_;
Ephemeris<Barycentric>::GeneralizedAdaptiveStepParameters
generalized_adaptive_step_parameters_;
Rotation<Barycentric, AliceSun> identity_;
std::unique_ptr<MockManœuvre<Barycentric, Navigation>> navigation_manœuvre_;
MockRenderer renderer_;
StrictMock<MockFlightPlan> flight_plan_;
not_null<std::unique_ptr<StrictMock<MockPlugin>>> const plugin_;
StrictMock<MockPlugin> const* const const_plugin_;
Instant const t0_;
Expand All @@ -129,7 +151,6 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
/*delta_v=*/{4, 5, 6},
/*is_inertially_fixed=*/true};
StrictMock<MockVessel> vessel;
StrictMock<MockFlightPlan> flight_plan;

EXPECT_CALL(*plugin_, HasVessel(vessel_guid))
.WillRepeatedly(Return(true));
Expand All @@ -138,7 +159,7 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
EXPECT_CALL(vessel, has_flight_plan())
.WillRepeatedly(Return(true));
EXPECT_CALL(vessel, flight_plan())
.WillRepeatedly(ReturnRef(flight_plan));
.WillRepeatedly(ReturnRef(flight_plan_));
EXPECT_CALL(*plugin_, ExtendPredictionForFlightPlan(vessel_guid))
.Times(AnyNumber());

Expand All @@ -152,23 +173,23 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
/*final_time=*/30,
/*mass_in_tonnes=*/100);

EXPECT_CALL(flight_plan, SetDesiredFinalTime(Instant() + 60 * Second))
EXPECT_CALL(flight_plan_, SetDesiredFinalTime(Instant() + 60 * Second))
.WillOnce(Return(absl::OkStatus()));
EXPECT_THAT(
*principia__FlightPlanSetDesiredFinalTime(plugin_.get(), vessel_guid, 60),
IsOk());

EXPECT_CALL(flight_plan, initial_time())
EXPECT_CALL(flight_plan_, initial_time())
.WillOnce(Return(Instant() + 3 * Second));
EXPECT_EQ(3, principia__FlightPlanGetInitialTime(plugin_.get(), vessel_guid));

EXPECT_CALL(flight_plan, desired_final_time())
EXPECT_CALL(flight_plan_, desired_final_time())
.WillOnce(Return(Instant() + 4 * Second));
EXPECT_EQ(4, principia__FlightPlanGetDesiredFinalTime(plugin_.get(),
vessel_guid));

EXPECT_CALL(
flight_plan,
flight_plan_,
SetAdaptiveStepParameters(
AllOf(
Property(
Expand Down Expand Up @@ -203,25 +224,10 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
/*speed_integration_tolerance=*/33}),
IsOk());

Ephemeris<Barycentric>::AdaptiveStepParameters adaptive_step_parameters(
EmbeddedExplicitRungeKuttaNyströmIntegrator<
DormandالمكاوىPrince1986RKN434FM,
Ephemeris<Barycentric>::NewtonianMotionEquation>(),
/*max_steps=*/111,
/*length_integration_tolerance=*/222 * Metre,
/*speed_integration_tolerance=*/333 * Metre / Second);
EXPECT_CALL(flight_plan, adaptive_step_parameters())
.WillOnce(ReturnRef(adaptive_step_parameters));
Ephemeris<Barycentric>::GeneralizedAdaptiveStepParameters
generalized_adaptive_step_parameters(
EmbeddedExplicitGeneralizedRungeKuttaNyströmIntegrator<
Fine1987RKNG34,
Ephemeris<Barycentric>::GeneralizedNewtonianMotionEquation>(),
/*max_steps=*/111,
/*length_integration_tolerance=*/222 * Metre,
/*speed_integration_tolerance=*/333 * Metre / Second);
EXPECT_CALL(flight_plan, generalized_adaptive_step_parameters())
.WillOnce(ReturnRef(generalized_adaptive_step_parameters));
EXPECT_CALL(flight_plan_, adaptive_step_parameters())
.WillOnce(ReturnRef(adaptive_step_parameters_));
EXPECT_CALL(flight_plan_, generalized_adaptive_step_parameters())
.WillOnce(ReturnRef(generalized_adaptive_step_parameters_));
FlightPlanAdaptiveStepParameters expected_adaptive_step_parameters = {
/*integrator_kind=*/1,
/*generalized_integrator_kind=*/2,
Expand All @@ -238,7 +244,7 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
ByMove(std::make_unique<StrictMock<
MockRigidReferenceFrame<Barycentric, Navigation>>>())));
EXPECT_CALL(
flight_plan,
flight_plan_,
Insert(AllOf(HasThrust(1 * Kilo(Newton)),
HasSpecificImpulse(2 * Second * StandardGravity),
HasInitialTime(Instant() + 3 * Second),
Expand All @@ -251,7 +257,7 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
plugin_.get(), vessel_guid, interface_burn, 0),
IsOk());

EXPECT_CALL(flight_plan, number_of_manœuvres())
EXPECT_CALL(flight_plan_, number_of_manœuvres())
.WillOnce(Return(4));
EXPECT_EQ(4, principia__FlightPlanNumberOfManoeuvres(plugin_.get(),
vessel_guid));
Expand Down Expand Up @@ -285,22 +291,21 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
std::unique_ptr<RigidReferenceFrame<Barycentric, Navigation> const>(
navigation_manœuvre_frame),
/*is_inertially_fixed=*/true};
MockManœuvre<Barycentric, Navigation> navigation_manœuvre(20 * Tonne, burn);
navigation_manœuvre_ =
std::make_unique<MockManœuvre<Barycentric, Navigation>>(20 * Tonne, burn);
auto const barycentric_to_plotting = RigidMotion<Barycentric, Navigation>(
RigidTransformation<Barycentric, Navigation>(
Barycentric::origin,
Navigation::origin,
OrthogonalMap<Barycentric, Navigation>::Identity()),
Barycentric::nonrotating,
Barycentric::unmoving);
MockRenderer renderer;
auto const identity = Rotation<Barycentric, AliceSun>::Identity();
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer));
EXPECT_CALL(*const_plugin_, renderer()).WillRepeatedly(ReturnRef(renderer));
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer_));
EXPECT_CALL(*const_plugin_, renderer()).WillRepeatedly(ReturnRef(renderer_));
EXPECT_CALL(*plugin_, PlanetariumRotation())
.WillRepeatedly(ReturnRef(identity));
EXPECT_CALL(flight_plan, GetManœuvre(3))
.WillOnce(ReturnRef(navigation_manœuvre));
.WillRepeatedly(ReturnRef(identity_));
EXPECT_CALL(flight_plan_, GetManœuvre(3))
.WillOnce(ReturnRef(*navigation_manœuvre_));
EXPECT_CALL(*plugin_, CelestialIndexOfBody(Ref(centre)))
.WillOnce(Return(celestial_index));
auto const navigation_manoeuvre =
Expand All @@ -316,20 +321,20 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
EXPECT_THAT(navigation_manoeuvre->burn.specific_impulse_in_seconds_g0,
AlmostEquals(30, 1));

EXPECT_CALL(flight_plan, GetManœuvre(3))
.WillOnce(ReturnRef(navigation_manœuvre));
EXPECT_CALL(flight_plan_, GetManœuvre(3))
.WillOnce(ReturnRef(*navigation_manœuvre_));

EXPECT_CALL(renderer, BarycentricToWorldSun(_))
EXPECT_CALL(renderer_, BarycentricToWorldSun(_))
.WillOnce(Return(
Permutation<Barycentric, WorldSun>(
Permutation<Barycentric, WorldSun>::CoordinatePermutation::YXZ)
.Forget<OrthogonalMap>()));
EXPECT_CALL(navigation_manœuvre, FrenetFrame())
EXPECT_CALL(*navigation_manœuvre_, FrenetFrame())
.WillOnce(
Return(OrthogonalMap<Frenet<Navigation>, Barycentric>::Identity()));
EXPECT_CALL(*plugin_, CurrentTime())
.WillRepeatedly(Return(Instant() - 4 * Second));
EXPECT_CALL(renderer, GetPlottingFrame())
EXPECT_CALL(renderer_, GetPlottingFrame())
.WillRepeatedly(Return(plotting_frame.get()));
EXPECT_CALL(*plotting_frame, ToThisFrameAtTime(Instant()))
.WillOnce(Return(barycentric_to_plotting));
Expand All @@ -339,7 +344,7 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
vessel_guid,
3);

EXPECT_CALL(flight_plan, number_of_segments())
EXPECT_CALL(flight_plan_, number_of_segments())
.WillOnce(Return(12));
EXPECT_EQ(12, principia__FlightPlanNumberOfSegments(plugin_.get(),
vessel_guid));
Expand All @@ -365,9 +370,9 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
EXPECT_OK(segment.Append(t0_, immobile_origin));
EXPECT_OK(segment.Append(t0_ + 1 * Second, immobile_origin));
EXPECT_OK(segment.Append(t0_ + 2 * Second, immobile_origin));
EXPECT_CALL(flight_plan, GetSegment(3))
EXPECT_CALL(flight_plan_, GetSegment(3))
.WillOnce(Return(segment.segments().begin()));
EXPECT_CALL(renderer, RenderBarycentricTrajectoryInWorld(_, _, _, _, _))
EXPECT_CALL(renderer_, RenderBarycentricTrajectoryInWorld(_, _, _, _, _))
.WillOnce(Return(ByMove(std::move(rendered_trajectory))));
auto* const iterator =
principia__FlightPlanRenderedSegment(plugin_.get(),
Expand All @@ -391,7 +396,7 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
MockRigidReferenceFrame<Barycentric, Navigation>>>())));
auto const manœuvre = NavigationManœuvre(/*initial_mass=*/1 * Kilogram, burn);
EXPECT_CALL(
flight_plan,
flight_plan_,
Replace(
AllOf(HasThrust(10 * Kilo(Newton)),
HasSpecificImpulse(2 * Second * StandardGravity),
Expand All @@ -405,7 +410,7 @@ TEST_F(InterfaceFlightPlanTest, FlightPlan) {
plugin_.get(), vessel_guid, interface_burn, 42),
IsOk());

EXPECT_CALL(flight_plan, Remove(0));
EXPECT_CALL(flight_plan_, Remove(0));
principia__FlightPlanRemove(plugin_.get(), vessel_guid, 0);

EXPECT_CALL(vessel, DeleteFlightPlan());
Expand Down
13 changes: 7 additions & 6 deletions ksp_plugin_test/interface_planetarium_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,22 +43,23 @@ using namespace principia::quantities::_quantities;
class InterfacePlanetariumTest : public ::testing::Test {
protected:
InterfacePlanetariumTest()
: plugin_(make_not_null_unique<StrictMock<MockPlugin>>()),
: identity_(Rotation<Barycentric, AliceSun>::Identity()),
plugin_(make_not_null_unique<StrictMock<MockPlugin>>()),
const_plugin_(plugin_.get()) {}

Rotation<Barycentric, AliceSun> identity_;
MockRenderer renderer_;
not_null<std::unique_ptr<StrictMock<MockPlugin>>> const plugin_;
StrictMock<MockPlugin> const* const const_plugin_;
Instant const t0_;
};

TEST_F(InterfacePlanetariumTest, ConstructionDestruction) {
auto const identity = Rotation<Barycentric, AliceSun>::Identity();
MockRenderer renderer;
EXPECT_CALL(*const_plugin_, renderer()).WillRepeatedly(ReturnRef(renderer));
EXPECT_CALL(*const_plugin_, renderer()).WillRepeatedly(ReturnRef(renderer_));
EXPECT_CALL(*plugin_, CurrentTime()).WillRepeatedly(Return(t0_));
EXPECT_CALL(*plugin_, PlanetariumRotation())
.WillRepeatedly(ReturnRef(identity));
EXPECT_CALL(renderer, WorldToPlotting(_, _, _))
.WillRepeatedly(ReturnRef(identity_));
EXPECT_CALL(renderer_, WorldToPlotting(_, _, _))
.WillOnce(Return(RigidTransformation<World, Navigation>(
World::origin,
Navigation::origin,
Expand Down
13 changes: 6 additions & 7 deletions ksp_plugin_test/interface_renderer_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class InterfaceRendererTest : public ::testing::Test {
: plugin_(make_not_null_unique<StrictMock<MockPlugin>>()),
const_plugin_(plugin_.get()) {}

MockRenderer renderer_;
not_null<std::unique_ptr<StrictMock<MockPlugin>>> const plugin_;
StrictMock<MockPlugin> const* const const_plugin_;
Instant const t0_;
Expand All @@ -75,10 +76,9 @@ TEST_F(InterfaceRendererTest, SetPlottingFrame) {
unused,
&celestial_index_array[0],
&parent_index_array[0]};
MockRenderer renderer;
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer));
EXPECT_CALL(*const_plugin_, renderer()).WillRepeatedly(ReturnRef(renderer));
EXPECT_CALL(renderer, SetPlottingFrame(Pointer(mock_navigation_frame)));
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer_));
EXPECT_CALL(*const_plugin_, renderer()).WillRepeatedly(ReturnRef(renderer_));
EXPECT_CALL(renderer_, SetPlottingFrame(Pointer(mock_navigation_frame)));
principia__SetPlottingFrame(plugin_.get(), parameters);
}

Expand All @@ -101,9 +101,8 @@ TEST_F(InterfaceRendererTest, Frenet) {
&celestial_index_array[0],
&parent_index_array[0]};

MockRenderer renderer;
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer));
EXPECT_CALL(renderer, SetPlottingFrame(Pointer(mock_navigation_frame)));
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer_));
EXPECT_CALL(renderer_, SetPlottingFrame(Pointer(mock_navigation_frame)));
principia__SetPlottingFrame(plugin_.get(), parameters);

{
Expand Down
13 changes: 6 additions & 7 deletions ksp_plugin_test/interface_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ class InterfaceTest : public testing::Test {
serialized_simple_plugin_(ReadFromBinaryFile(
SOLUTION_DIR / "ksp_plugin_test" / "simple_plugin.proto.bin")) {}

MockRenderer renderer_;
not_null<std::unique_ptr<StrictMock<MockPlugin>>> plugin_;
std::string const hexadecimal_simple_plugin_;
std::vector<std::uint8_t> const serialized_simple_plugin_;
Expand Down Expand Up @@ -522,8 +523,7 @@ TEST_F(InterfaceTest, CelestialFromParent) {
}

TEST_F(InterfaceTest, NewNavigationFrame) {
MockRenderer renderer;
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer));
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer_));

int const* const celestial_index_array[2] = {&celestial_index, nullptr};
int const* const parent_index_array[2] = {&parent_index, nullptr};
Expand All @@ -543,7 +543,7 @@ TEST_F(InterfaceTest, NewNavigationFrame) {
std::unique_ptr<
StrictMock<MockRigidReferenceFrame<Barycentric, Navigation>>>(
mock_navigation_frame))));
EXPECT_CALL(renderer, SetPlottingFrame(Pointer(mock_navigation_frame)));
EXPECT_CALL(renderer_, SetPlottingFrame(Pointer(mock_navigation_frame)));
principia__SetPlottingFrame(plugin_.get(), parameters);
}

Expand All @@ -560,7 +560,7 @@ TEST_F(InterfaceTest, NewNavigationFrame) {
std::unique_ptr<
StrictMock<MockRigidReferenceFrame<Barycentric, Navigation>>>(
mock_navigation_frame))));
EXPECT_CALL(renderer, SetPlottingFrame(Pointer(mock_navigation_frame)));
EXPECT_CALL(renderer_, SetPlottingFrame(Pointer(mock_navigation_frame)));
principia__SetPlottingFrame(plugin_.get(), parameters);
}
}
Expand All @@ -584,9 +584,8 @@ TEST_F(InterfaceTest, NavballOrientation) {
&celestial_index_array[0],
&parent_index_array[0]};

MockRenderer renderer;
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer));
EXPECT_CALL(renderer, SetPlottingFrame(Pointer(mock_navigation_frame)));
EXPECT_CALL(*plugin_, renderer()).WillRepeatedly(ReturnRef(renderer_));
EXPECT_CALL(renderer_, SetPlottingFrame(Pointer(mock_navigation_frame)));
principia__SetPlottingFrame(plugin_.get(), parameters);

Position<World> sun_position =
Expand Down
1 change: 0 additions & 1 deletion ksp_plugin_test/pile_up_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ using ::testing::IsEmpty;
using ::testing::Matcher;
using ::testing::MockFunction;
using ::testing::Return;
using ::testing::ReturnRef;
using ::testing::_;
using namespace principia::astronomy::_epoch;
using namespace principia::base::_not_null;
Expand Down
1 change: 0 additions & 1 deletion ksp_plugin_test/plugin_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ using ::testing::Lt;
using ::testing::Ne;
using ::testing::Ref;
using ::testing::Return;
using ::testing::ReturnRef;
using ::testing::SaveArg;
using ::testing::SetArgPointee;
using ::testing::SizeIs;
Expand Down
10 changes: 5 additions & 5 deletions ksp_plugin_test/vessel_test.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,9 +140,11 @@ class VesselTest : public testing::Test {
vessel_.checkpointer_->WriteToMessage(message->mutable_checkpoint());
}

MockEphemeris<Barycentric> ephemeris_;
std::vector<not_null<MassiveBody const*>> const bodies_;
RotatingBody<Barycentric> const body_;
Celestial const celestial_;
MockEphemeris<Barycentric> ephemeris_;

PartId const part_id1_ = 111;
PartId const part_id2_ = 222;
Mass const mass1_ = 1 * Kilogram;
Expand Down Expand Up @@ -398,8 +400,7 @@ TEST_F(VesselTest, FlightPlan) {
EXPECT_CALL(ephemeris_,
Prolong(_, _))
.Times(AnyNumber());
std::vector<not_null<MassiveBody const*>> const bodies;
ON_CALL(ephemeris_, bodies()).WillByDefault(ReturnRef(bodies));
ON_CALL(ephemeris_, bodies()).WillByDefault(ReturnRef(bodies_));
vessel_.CreateTrajectoryIfNeeded(t0_);

EXPECT_FALSE(vessel_.has_flight_plan());
Expand Down Expand Up @@ -822,8 +823,7 @@ TEST_F(VesselTest, SerializationSuccess) {
Prolong(_, _))
.WillRepeatedly(Return(absl::OkStatus()));

std::vector<not_null<MassiveBody const*>> const bodies;
ON_CALL(ephemeris_, bodies()).WillByDefault(ReturnRef(bodies));
ON_CALL(ephemeris_, bodies()).WillByDefault(ReturnRef(bodies_));

vessel_.CreateFlightPlan(t0_ + 3.0 * Second,
10 * Kilogram,
Expand Down

0 comments on commit 4f09547

Please sign in to comment.