-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add virtual destructor to avoid compiler warning on undefined behavior #557
Conversation
Memory usage change @ be75fe7
Click for full report table
Click for full report CSV
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your pull request @gillesdegottex!
There is a related discussion here:
You can see here that the equivalent change was originally also proposed in arduino/ArduinoCore-API#218:
--- a/api/Server.h
+++ b/api/Server.h
@@ -25,6 +25,7 @@ namespace arduino {
class Server : public Print {
public:
+ virtual ~Server() {}
virtual void begin() = 0;
};
However, the proposal was adjusted per this comment:
arduino/ArduinoCore-API#218 (comment)
It is enough to add virtual destructor to
Stream
,Client
,Server
,HardwareI2C
andHardwareSerial
are inherited from
Oh, my bad, I didn't see this similar PR. I'll close mine then and see if I can bring any 2 cents useful for API#218. |
No worries. The goal of my comment was only to make sure the related work in the other repository was given consideration (I don't have enough expertise in C++ to make a judgement in the specific approach that should be taken in this matter). The intent behind the |
Thx for the detailed answer! |
The
Server
class is virtual (begin()
is not defined).Standard thus requires to have the destructor virtual, otherwise undefined behaviour is expected.
This PR suggest to fix this issue by adding a simple empty destructor.