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

[POC RFC] header only API for singletons (Windows) #1595

Closed
wants to merge 15 commits into from
Closed

[POC RFC] header only API for singletons (Windows) #1595

Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
ef23861
WIP
marcalff Sep 6, 2022
d231197
WIP
marcalff Sep 6, 2022
7c52d4b
Testing tentative fix for windows
marcalff Sep 6, 2022
6a8f2ff
Disable unit test for windows
marcalff Sep 6, 2022
c81cc34
Partial revert of:
marcalff Sep 7, 2022
84d6bab
format
marcalff Sep 7, 2022
5de5b6a
format
marcalff Sep 7, 2022
cbf70a7
Alternate implementation for windows.
marcalff Sep 7, 2022
5aeeb74
Fixed windows build break.
marcalff Sep 7, 2022
c6a8970
Merge branch 'open-telemetry:main' into poc_api_singletons_1520_b
marcalff Sep 7, 2022
556b592
Fix code review comments,
marcalff Sep 8, 2022
a4fa08e
Merge branch 'open-telemetry:main' into poc_api_singletons_1520_b
marcalff Sep 8, 2022
b5b6552
Tentative, to test on CI:
marcalff Sep 8, 2022
868b6b0
Tentative,
marcalff Sep 9, 2022
4d11eac
Merge branch 'main' into poc_api_singletons_1520_b
marcalff Sep 19, 2022
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: 2 additions & 1 deletion api/include/opentelemetry/common/macros.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@
#elif defined(__GNUC__)
# define OPENTELEMETRY_API_SINGLETON __attribute__((visibility("default")))
#elif defined(_MSC_VER)
# define OPENTELEMETRY_API_SINGLETON
/* Tentative fix */
# define OPENTELEMETRY_API_SINGLETON __declspec(selectany)
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering why __declspec(selectany) is removed?Doesn't it work with MSVC on Windows?

Copy link
Member Author

Choose a reason for hiding this comment

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

@owent

__declspec(selectany) works when applied to a member.

This PR tested applying it to a method, which failed to compile.

See places where OPENTELEMETRY_API_SINGLETON is used in this patch,
which are different compared to PR#1525

Copy link
Member

Choose a reason for hiding this comment

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

Could we also add __declspec(selectany) to variables, so we can also support dll on Windows with MSVC?

Copy link
Member Author

Choose a reason for hiding this comment

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

@owent, see file macros.h in the latest push.

#else
# define OPENTELEMETRY_API_SINGLETON
#endif