Skip to content

Give FFPoint one way to copy itself - #191

Merged
antonio-leblanc merged 1 commit into
devfrom
refactor/ffpoint-copy-assignment
Aug 17, 2026
Merged

Give FFPoint one way to copy itself#191
antonio-leblanc merged 1 commit into
devfrom
refactor/ffpoint-copy-assignment

Conversation

@HugoFara

@HugoFara HugoFara commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

FFPoint is the type every coordinate in the simulation passes through, and copy is defined ambiguously with assignement.

FFPoint::FFPoint(const FFPoint& p) : x(p.x), y(p.y), z(p.z) { }   // misses it
// b = a                                                                                            // copies it

FFPoint b(a); and b = a; then produce different objects.

So the assignment operator is written out, directly below the copy-constructor, where a future member has to be added to both or the omission is visible in one file. FFVector has the identical shape and gets the identical treatment. Behaviour is unchanged: the member-wise copy written here is exactly what the compiler was generating.

Verification

Unit suite 4/4, runff KML and NetCDF both match within tolerance.

One thing noticed and deliberately left alone: nothing derives from either class, so those virtual destructors cost every FFPoint a vptr for nothing. Removing them is a future decision.

Contributes to #161.


This pull request, including its code changes and this description, was generated by Claude Opus 5, and reviewed manually before submitting.

Both hand-write a copy-constructor and leave assignment to the compiler,
so each class carries two independently maintained definitions of how to
copy itself. They agree today. They stop agreeing the moment anyone adds
a member: the generated assignment picks it up, the hand-written
constructor does not, leaving `FFPoint b(a)` and `b = a` producing
different objects. FFPoint carries every coordinate in the simulation.

Writing the assignment operator out puts both next to each other, so a
new member has to be added to both or the omission is visible in one
file. Behaviour is unchanged: three doubles each, owning nothing.

Contributes to #161, which is where this came to notice: 65 of its 428
warning lines were the missing operator. The issue attributes all 65 to
FFPoint; 51 are, the other 14 are FFVector, which has the same shape.

@antonio-leblanc antonio-leblanc left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. Went through both classes fully, not just the diff — FFPoint/FFVector only hold doubles, no owned resources, so this operator= is byte-for-byte what the compiler was already synthesizing implicitly.

Behavior's unchanged, and since nothing here touches ROS/propagation logic, no physics impact either — this is pure C++ hygiene, just silences -Wdeprecated-copy.

Safe to merge.

@antonio-leblanc
antonio-leblanc merged commit 7467f40 into dev Aug 17, 2026
4 checks passed
@antonio-leblanc
antonio-leblanc deleted the refactor/ffpoint-copy-assignment branch August 17, 2026 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants