Skip to content

Progress: enforce rule-of-five (currently default copy → double-pop) #120

Description

@gagelarsen

Follow-up from issue #116 (review finding #10).

xmscore/misc/Progress.h:23-39Progress is documented as an RAII helper that pushes a slot in its ctor and pops it in its dtor, but it leaves the implicit copy ctor / copy-assign generated. A copy duplicates m_stackIndex; both copies pop on destruction → double-pop on the listener stack.

```cpp
class Progress {
public:
Progress(...);
~Progress(); // pops the progress stack
private:
int m_stackIndex; // copyable by default
};
```

Proposed change

Pick one:

  1. Delete copy/move (simplest, matches docs):
    ```cpp
    Progress(const Progress&) = delete;
    Progress& operator=(const Progress&) = delete;
    Progress(Progress&&) = delete;
    Progress& operator=(Progress&&) = delete;
    ```
  2. Rule-of-zero via pimpl: hold std::unique_ptr<Impl> so the impl owns the stack slot; the implicit special members then do the right thing.

Why follow-up, not part of #117

The new \brief in #117 freezes "RAII helper" as the contract while the type silently breaks it. Picking between (1) and (2) is a real type-design call, not a doc fix.

Acceptance

  • Progress cannot be copied (or, if copies are kept, copying does not produce duplicate stack slots)
  • Existing call sites still compile (or are migrated)
  • CxxTest coverage for the previously-broken double-pop case

Refs: issue #116, PR #117.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions