Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[AVRO-3942] Mark MemoryOutputStream as final #2752

Merged
merged 1 commit into from
Feb 20, 2024

Conversation

mkmkme
Copy link
Contributor

@mkmkme mkmkme commented Feb 20, 2024

AVRO-3942

What is the purpose of the change

Otherwise the class raises a compilation warning:

lang/c++/impl/Stream.cc:129:27: warning: class with destructor marked 'final' cannot be inherited from [-Wfinal-dtor-non-final-class]
  120 | class MemoryOutputStream : public OutputStream {
      |                          final
  121 | public:
  122 |     const size_t chunkSize_;
  123 |     std::vector<uint8_t *> data_;
  124 |     size_t available_;
  125 |     size_t byteCount_;
  126 |
  127 |     explicit MemoryOutputStream(size_t chunkSize) : chunkSize_(chunkSize),
  128 |                                                     available_(0), byteCount_(0) {}
  129 |     ~MemoryOutputStream() final {
      |                           ^
lang/c++/impl/Stream.cc:120:7: note: mark 'avro::MemoryOutputStream' as 'final' to silence this warning
  120 | class MemoryOutputStream : public OutputStream {
      |       ^

Verifying this change

This change is a trivial rework / code cleanup without any test coverage.

Documentation

  • Does this pull request introduce a new feature? no

Otherwise the class raises a compilation warning:

lang/c++/impl/Stream.cc:129:27: warning: class with destructor marked 'final' cannot be inherited from [-Wfinal-dtor-non-final-class]
  120 | class MemoryOutputStream : public OutputStream {
      |                          final
  121 | public:
  122 |     const size_t chunkSize_;
  123 |     std::vector<uint8_t *> data_;
  124 |     size_t available_;
  125 |     size_t byteCount_;
  126 |
  127 |     explicit MemoryOutputStream(size_t chunkSize) : chunkSize_(chunkSize),
  128 |                                                     available_(0), byteCount_(0) {}
  129 |     ~MemoryOutputStream() final {
      |                           ^
lang/c++/impl/Stream.cc:120:7: note: mark 'avro::MemoryOutputStream' as 'final' to silence this warning
  120 | class MemoryOutputStream : public OutputStream {
      |       ^
@github-actions github-actions bot added the C++ Pull Requests for C++ binding label Feb 20, 2024
@mkmkme
Copy link
Contributor Author

mkmkme commented Feb 20, 2024

This PR is effectively the same as #2550 which got closed because the fork was deleted.

Copy link
Contributor

@KalleOlaviNiemitalo KalleOlaviNiemitalo left a comment

Choose a reason for hiding this comment

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

OK, this file already uses final in function members, and both were introduced in C++11.

This change will cause std::is_final<MemoryOutputStream>::value to become true but I expect that will only be a good thing.

@martin-g martin-g merged commit 258571b into apache:main Feb 20, 2024
4 checks passed
martin-g pushed a commit that referenced this pull request Feb 20, 2024
Otherwise the class raises a compilation warning:

lang/c++/impl/Stream.cc:129:27: warning: class with destructor marked 'final' cannot be inherited from [-Wfinal-dtor-non-final-class]
  120 | class MemoryOutputStream : public OutputStream {
      |                          final
  121 | public:
  122 |     const size_t chunkSize_;
  123 |     std::vector<uint8_t *> data_;
  124 |     size_t available_;
  125 |     size_t byteCount_;
  126 |
  127 |     explicit MemoryOutputStream(size_t chunkSize) : chunkSize_(chunkSize),
  128 |                                                     available_(0), byteCount_(0) {}
  129 |     ~MemoryOutputStream() final {
      |                           ^
lang/c++/impl/Stream.cc:120:7: note: mark 'avro::MemoryOutputStream' as 'final' to silence this warning
  120 | class MemoryOutputStream : public OutputStream {
      |       ^

(cherry picked from commit 258571b)
@martin-g
Copy link
Member

Thank you, @mkmkme !

RanbirK pushed a commit to RanbirK/avro that referenced this pull request May 13, 2024
Otherwise the class raises a compilation warning:

lang/c++/impl/Stream.cc:129:27: warning: class with destructor marked 'final' cannot be inherited from [-Wfinal-dtor-non-final-class]
  120 | class MemoryOutputStream : public OutputStream {
      |                          final
  121 | public:
  122 |     const size_t chunkSize_;
  123 |     std::vector<uint8_t *> data_;
  124 |     size_t available_;
  125 |     size_t byteCount_;
  126 |
  127 |     explicit MemoryOutputStream(size_t chunkSize) : chunkSize_(chunkSize),
  128 |                                                     available_(0), byteCount_(0) {}
  129 |     ~MemoryOutputStream() final {
      |                           ^
lang/c++/impl/Stream.cc:120:7: note: mark 'avro::MemoryOutputStream' as 'final' to silence this warning
  120 | class MemoryOutputStream : public OutputStream {
      |       ^
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C++ Pull Requests for C++ binding
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants