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.

Sign up for GitHub

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

Export Data3DPointsData_t class in shared library. #252

Closed
wants to merge 1 commit into from
Closed

Export Data3DPointsData_t class in shared library. #252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions include/E57SimpleData.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -744,9 +744,6 @@ namespace e57
using Data3DPointsData_d [[deprecated( "Will be removed in 4.0. Use Data3DPointsDouble." )]] =
Data3DPointsData_t<double>;

extern template struct Data3DPointsData_t<float>;
extern template struct Data3DPointsData_t<double>;

Copy link
Owner

Choose a reason for hiding this comment

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

This change removes the whole reason for having explicit template instantiation in the cpp file.

What error is the original code giving you? What compiler are you using?

According to everything I looked at w.r.t. Windows said that the way I did it is correct and the CI tests run properly with the shared lib. Maybe there's some other issue that's showing up as a problem with this declaration?

Copy link
Owner

Choose a reason for hiding this comment

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

Quick follow-up. I just tried it locally with this compiler:

C:/Program Files (x86)/Microsoft Visual Studio/2019/Community/VC/Tools/MSVC/14.29.30133/bin/HostX64/x64/cl.exe

The test executable builds and runs without warnings/errors when using E57Format as a shared lib. It uses both Data3DPointsFloat and Data3DPointsDouble.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, so my original problem is that Data3DPointsData_t<double> and Data3DPointsData_t<false> are hidden in Linux shared library. If I added E57_DLL at line 747 I got the following Windows build errors:

D:\a\libE57Format\libE57Format\include\E57SimpleData.h(747): error C2220: the following warning is treated as an error
D:\a\libE57Format\libE57Format\include\E57SimpleData.h(747): warning C4910: 'e57::Data3DPointsData_t<float>': '__declspec(dllexport)' and 'extern' are incompatible on an explicit instantiation
D:\a\libE57Format\libE57Format\include\E57SimpleData.h(748): warning C4910: 'e57::Data3DPointsData_t<double>': '__declspec(dllexport)' and 'extern' are incompatible on an explicit instantiation

which is about one can't combine extern and __declspec(dllexport) at one line. And so I suggested to use E57_DLL in the cpp file.

Copy link
Author

Choose a reason for hiding this comment

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

I will verify that the symbol is hidden on Linux tomorrow.

/// @brief Stores an image that is to be used only as a visual reference.
struct E57_DLL VisualReferenceRepresentation
{
Expand Down
5 changes: 0 additions & 5 deletions src/E57SimpleData.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,11 +247,6 @@ namespace e57
*this = Data3DPointsData_t<COORDTYPE>();
}

#if defined( _MSC_VER )
template struct E57_DLL Data3DPointsData_t<float>;
template struct E57_DLL Data3DPointsData_t<double>;
#else
template struct Data3DPointsData_t<float>;
template struct Data3DPointsData_t<double>;
#endif
} // end namespace e57