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

Fix weak vtable warning regarding xml_writer #550

Merged
merged 1 commit into from
Apr 21, 2023
Merged

Fix weak vtable warning regarding xml_writer #550

merged 1 commit into from
Apr 21, 2023

Conversation

asmaloney
Copy link
Contributor

Using Apple clang (clang-1400.0.29.202) with -Wweak-vtables would produce the following warning:

'xml_writer' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Wweak-vtables]

Using the "defaulted default constructor" prevents this.

@asmaloney
Copy link
Contributor Author

asmaloney commented Mar 8, 2023

Oh - I missed that this failed CI. I didn't get any notification.

It looks like the really old MSVC 2015 doesn't fully support C++11?

c:\projects\pugixml\tests../src/pugixml.hpp(327) : error C2065: 'default' : undeclared identifier

@zeux
Copy link
Owner

zeux commented Apr 21, 2023

Yeah, pugixml doesn't require C++11. In this case it should be easy to move the empty definition of xml_writer dtor to pugixml.cpp.

@asmaloney
Copy link
Contributor Author

Ah thanks. I meant to do that, but I switched out to a different project and forgot about this. Will fix it.

Using Apple clang (clang-1400.0.29.202) with `-Wweak-vtables` would produce the following warning:

'xml_writer' has no out-of-line virtual method definitions; its vtable will be emitted in every translation unit [-Wweak-vtables]
@zeux zeux merged commit 8d18de8 into zeux:master Apr 21, 2023
@zeux
Copy link
Owner

zeux commented Apr 21, 2023

Thanks!

@asmaloney asmaloney deleted the fix-weak-vtable-warning branch April 21, 2023 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants