[ https://issues.apache.org/jira/browse/THRIFT-5678?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17679583#comment-17679583 ]
Mario Emmenlauer commented on THRIFT-5678: ------------------------------------------ Thanks a lot [~cfriedt] . I would have a tiny preference to have just the `virtual` keyword in front of the destructor in the header, and then add a "normal" destructor in the implementation file. This is because some C++ parsers do not yet support the `= default` keywords. Its not a strong recommendation, just my personal preference. > TConnectedClient: warning due to non-virtual dtor > ------------------------------------------------- > > Key: THRIFT-5678 > URL: https://issues.apache.org/jira/browse/THRIFT-5678 > Project: Thrift > Issue Type: Bug > Components: C++ - Library > Affects Versions: 0.13.0, 0.14.0, 0.15.0, 0.14.1, 0.14.2, 0.16.0, 0.17.0 > Reporter: Christopher Friedt > Priority: Minor > Time Spent: 20m > Remaining Estimate: 0h > > In TConnectedClient.h the destructor is non-virtual, which causes the warning > below. > {code:java} > In member function 'void > apache::thrift::server::TServerFramework::disposeConnectedClient(apache::thrift::server::TConnectedClient*)': > lib/cpp/src/thrift/server/TServerFramework.cpp:234:3: warning: deleting > object of polymorphic class type 'apache::thrift::server::TConnectedClient' > which has non-virtual destructor might cause undefined behavior > [-Wdelete-non-virtual-dtor] > 234 | delete pClient; > | ^~~~~~~~~~~~~~{code} > Not itself a major problem, but Zephyr CI promotes all warnings to errors, > which causes builds to fail. > TConnectedClient.cpp has the following line of code: > {code:java} > TConnectedClient::~TConnectedClient() = default;{code} > It might be considered a regression. The commit that removed the virtual > qualifier was 042580f53441efe1bc5c80c89351fcb30740659e > Suggested fix is to mark it as virtual and set it to default in the header, > and then delete the definition in the .cpp file. > Alternatively, keep the .cpp dtor, but simply re-add the virtual keyword in > the header -- This message was sent by Atlassian Jira (v8.20.10#820010)