Skip to content

[VL] Add virtual destructor in RowVectorStream#11228

Merged
jinchengchenghh merged 1 commit intoapache:mainfrom
Zouxxyy:dev/fix-mac-build
Dec 1, 2025
Merged

[VL] Add virtual destructor in RowVectorStream#11228
jinchengchenghh merged 1 commit intoapache:mainfrom
Zouxxyy:dev/fix-mac-build

Conversation

@Zouxxyy
Copy link
Copy Markdown
Contributor

@Zouxxyy Zouxxyy commented Nov 29, 2025

What changes are proposed in this pull request?

Fix the compile error on my mac

/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:78:5: error: delete called on non-final 'gluten::RowVectorStream' that has virtual functions but non-virtual destructor [-Werror,-Wdelete-non-abstract-non-virtual-dtor]
   78 |     delete __ptr;
      |     ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:300:7: note: in instantiation of member function 'std::default_delete<gluten::RowVectorStream>::operator()' requested here
  300 |       __deleter_(__tmp);
      |       ^
/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include/c++/v1/__memory/unique_ptr.h:269:71: note: in instantiation of member function 'std::unique_ptr<gluten::RowVectorStream>::reset' requested here
  269 |   _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 ~unique_ptr() { reset(); }
      |                                                                       ^
/project/emr/gluten/cpp/velox/operators/plannodes/RowVectorStream.h:92:3: note: in instantiation of member function 'std::unique_ptr<gluten::RowVectorStream>::~unique_ptr' requested here
   92 |   ValueStream(
      |   ^
1 error generated.
make[2]: *** [velox/CMakeFiles/velox.dir/compute/VeloxBackend.cc.o] Error 1
make[1]: *** [velox/CMakeFiles/velox.dir/all] Error 2
make: *** [all] Error 2

How was this patch tested?

@github-actions github-actions Bot added the VELOX label Nov 29, 2025
Copy link
Copy Markdown
Contributor

@jinchengchenghh jinchengchenghh left a comment

Choose a reason for hiding this comment

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

Thanks!

@jinchengchenghh jinchengchenghh merged commit 7d491d5 into apache:main Dec 1, 2025
60 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants