Skip to content

Minor warning under eclipse IDE (esp8266 core version 2.0) #1557

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

Closed
miky2k opened this issue Feb 2, 2016 · 6 comments
Closed

Minor warning under eclipse IDE (esp8266 core version 2.0) #1557

miky2k opened this issue Feb 2, 2016 · 6 comments
Labels
component: libraries level: easy type: bug waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Milestone

Comments

@miky2k
Copy link

miky2k commented Feb 2, 2016

Description Resource Path Location Type
Class 'WiFiUDP' has virtual method 'stop' but non-virtual destructor WiFiUdp.h /esp8266_rgbwebstrip/Libraries/ESP8266WiFi/src line 40 Code Analysis Problem

Description Resource Path Location Type
No return, in function returning non-void libc_replacements.c /esp8266_rgbwebstrip/arduino/core line 141 Code Analysis Problem

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@devyte
Copy link
Collaborator

devyte commented Oct 18, 2017

This is still valid in latest git. If there are virtual methods, then the destructor should also be virtual. If the class shouldn't allow inheritance, then the destructor should be protected.
A quick search in the repo shows that currently no class inherits from WiFiUdp.
@igrr I'm inclined towards removing all virtual decorators. What do you think?

@devyte devyte added component: libraries level: easy type: bug waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. labels Oct 18, 2017
@devyte
Copy link
Collaborator

devyte commented Oct 18, 2017

A quick repo search shows other classes with the same issue. I'd say we need to revisit all classes with virtual methods.

@igrr
Copy link
Member

igrr commented Oct 22, 2017

Many methods of WiFiUDP are virtual, because it inherits from UDP -> Stream -> Print.
To make things more obvious I would change declarations from

virtual void foo();

to

void foo() override;

in WiFiUDP. And also make the destructor virtual, of course.

@earlephilhower earlephilhower added this to the 2.6.0 milestone Jan 16, 2019
@earlephilhower
Copy link
Collaborator

Anyone want to volunteer to do a pass through the code for this? It's a good note.

@liebman
Copy link
Contributor

liebman commented Jan 17, 2019

If there is no rush I can take a look.

@d-a-v
Copy link
Collaborator

d-a-v commented Jan 19, 2019

Fixed by #5637, thanks!

@d-a-v d-a-v closed this as completed Jan 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: libraries level: easy type: bug waiting for feedback Waiting on additional info. If it's not received, the issue may be closed.
Projects
None yet
Development

No branches or pull requests

6 participants