Skip to content

Conversation

@jcstroud
Copy link

@jcstroud jcstroud commented Aug 16, 2024

Fixed Deprecated Copy Constructor Warning in polynomial.hpp

Issue Description

The boost/random/detail/polynomial.hpp triggers compiler warnings related to a deprecated implicit copy constructor:

warning: definition of implicit copy constructor for 'reference' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy]

This warning occurs in C++11 and later standards when a class has a user-declared copy assignment operator but no user-declared copy constructor.

Cause

The reference class has a user-declared copy assignment operator:

reference &operator=(const reference &other)

However, this reference class lacks an explicitly declared copy constructor.

Nature of the Fix

The fix explicitly declares both a copy constructor and a move constructor for reference, adding the following lines:

reference(const reference& other) = default;
reference(reference&& other) = default;

When declaring these constructors as default, the compiler generates default implementations, thus:

  1. Silencing the deprecation warning for the implicit copy constructor.
  2. Maintaining the existing behavior of the reference class.
  3. Ensuring that the class can still be copied and moved as before, preserving its functionality.

Impact of the Fix

This fix resolves the compiler warnings without changing the functionality of the reference class.

The fix is backwards-compatible and should not introduce any behavioral changes in existing code.

Conclusion

This update brings the polynomial.hpp file in line with current C++ standards and best practices. It eliminates deprecation warnings while maintaining existing functionality.

# Fixed Deprecated Copy Constructor Warning in `polynomial.hpp`

## Issue Description

The `boost/random/detail/polynomial.hpp` triggers compiler warnings related to a deprecated implicit copy constructor:

```
warning: definition of implicit copy constructor for 'reference' is deprecated because it has a user-declared copy assignment operator [-Wdeprecated-copy]
```

This warning occurs in C++11 and later standards when a class has a user-declared copy assignment operator but no user-declared copy constructor.

## Cause

The `reference` class has a user-declared copy assignment operator:

```cpp
reference &operator=(const reference &other)
```

However, this reference class lacks an explicitly declared copy constructor.

## Nature of the Fix

The fix explicitly declares both a copy constructor and a move constructor for `reference`, adding the following lines:

```cpp
reference(const reference& other) = default;
reference(reference&& other) = default;
```

When declaring these constructors as `default`, the compiler to generates default implementations, thus:

1. Silencing the deprecation warning for the implicit copy constructor.
2. Maintaining the existing behavior of the `reference` class.
3. Ensuring that the class can still be copied and moved as before, preserving its functionality.

## Impact of the Fix

This fix resolves the compiler warnings without changing the functionality of the `reference` class.

The fix is backwards-compatible and should not introduce any behavioral changes in existing code.

## Conclusion

This update brings the `polynomial.hpp` file in line with current C++ standards and best practices. It eliminates deprecation warnings while maintaining existing functionality.
@codecov
Copy link

codecov bot commented Aug 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.84%. Comparing base (560acd8) to head (21ba664).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #110   +/-   ##
========================================
  Coverage    94.84%   94.84%           
========================================
  Files          101      101           
  Lines         6242     6242           
========================================
  Hits          5920     5920           
  Misses         322      322           
Files Coverage Δ
include/boost/random/detail/polynomial.hpp 96.93% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 560acd8...21ba664. Read the comment docs.

@jcstroud
Copy link
Author

Many checks were not successful, so I'm going to close this pull request and make another with extended compiler backwards compatibility. I'll note the forwards-compatible fix in my next attempt.

@mborland
Copy link
Member

I believe this fix is correct. The issues that I see in the CI are coming from 1) Github breaking the Node version in Actions, and 2) The boost.math dependency makes it so this library needs to be built now from C++14 and not from C++11. I'll fix up the CI system.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants