Skip to content

Conversation

@nlhepler
Copy link

GCC pragmas aren't available (and visibility=hidden by default w/ VS), and __declspec must be all the way to the left.

@nlhepler nlhepler requested a review from adam-azarchs August 20, 2021 06:20
#pragma GCC visibility pop
#endif

#endif

Choose a reason for hiding this comment

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

This probably should have always been there but before there were other #if in here there wasn't anything to be confused about.

Suggested change
#endif
#endif // !defined(SPTREE_H)

@@ -53,7 +53,7 @@

Choose a reason for hiding this comment

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

I think in windows we also need to define it as dllimport when included from places which are not this binary. If we're only building this into a binary, not as a dll on windows, then we shouldn't be using dllexport.

Copy link
Author

Choose a reason for hiding this comment

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

Given how we're using this, it wasn't strictly necessary. But I did it anyway.

and visibility fixes for visual studio
@nlhepler
Copy link
Author

I'm going to go ahead and merge this, if there are additional changes you would like I can address them in a follow-up.

@nlhepler nlhepler merged commit 5f71f74 into lh/crana Aug 20, 2021
@nlhepler nlhepler deleted the lh/vs-fixes branch August 20, 2021 17:09
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.

3 participants