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

Define variants of WEBVIEW_API with new defaults #893

Merged
merged 12 commits into from
Sep 16, 2023

Conversation

SteffenL
Copy link
Collaborator

This PR is going to need some discussion to make sure that we are doing the right thing.

Proposal

This work provides new compile-time options intended to simplify compilation and integration of the library on the source code side. The default linkage was always extern which would cause ODR violations by default when the library's header was included in multiple C++ source files. The new linkage depends on the presence of compile-time options as well as whether the library's header file is included in C or C++ code.

The following use cases were taken into consideration:

  • Building a shared library (C++) and integrating a shared library (C/C++).
  • Building a static library (C++) and integrating a static library (C/C++).
  • Using the library as a header-only library (C++).

Options

  • WEBVIEW_SHARED should be defined when using the library as a shared library (C/C++).

  • WEBVIEW_BUILD_SHARED should be defined when building the library as a shared library (C++).

  • WEBVIEW_STATIC should be defined when building or using the library as a static library (C/C++).

  • If none of the above are defined then defaults will be used.

All of the mentioned options are mutually exclusive.

Details

  • When WEBVIEW_API is already defined then the linkage is as specified by WEBVIEW_API.

  • When webview.h is included in C++ code without any options then WEBVIEW_API is set to inline to avoid ODR violations. Existing users who rely on WEBVIEW_API to default to extern may see undefined references unless they define either WEBVIEW_STATIC or WEBVIEW_API (as extern or nothing).

  • When webview.h is included in C code without any options then WEBVIEW_API is set to nothing which defaults to extern as before.

  • When WEBVIEW_STATIC is defined then WEBVIEW_API is set to nothing (extern).

  • When WEBVIEW_BUILD_SHARED is defined then WEBVIEW_API is set to __declspec(dllexport) when building for Windows and otherwise __attribute__((visibility("default"))).

  • When WEBVIEW_SHARED is defined then WEBVIEW_API is set to __declspec(dllimport) for Windows and otherwise __attribute__((visibility("default"))).

Discussion Points

  • Should we provide the new compile-time options at all or instead educate people on correct usage of WEBVIEW_API?
  • Changing the default linking for the C API is not ideal but are there better options?
  • Are the macro names (WEBVIEW_BUILD_SHARED, WEBVIEW_SHARED, WEBVIEW_STATIC) appropriate?
  • How do the changes affect existing bindings?
  • Should we make an official release before merging this work?
  • WEBVIEW_STATIC is defined for Go bindings so that external linkage is used, just like before. Should we discuss whether Go bindings should keep using a static library by default or change it to shared library, or make that an option somehow? We have to consider the inconvenience with MS WebView2 on Windows.

This work provides new compile-time options intended to simplify compilation
and integration of the library on the source code side.

The default linkage was always extern which would cause ODR violations by
default when the library's header was included in multiple C++ source files.

The new linkage depends on the presence of compile-time options as well as
whether the library's header file is included in C or C++ code.

The following use cases were taken into consideration:

* Building a shared library (C++) and integrating a shared library (C/C++).
* Building a static library (C++) and integrating a static library (C/C++).
* Using the library as a header-only library (C++).

Options:

* WEBVIEW_SHARED should be defined when using the library as a shared
  library (C/C++).

* WEBVIEW_BUILD_SHARED should be defined when building the library as a
  shared library (C++).

* WEBVIEW_STATIC should be defined when building or using the library as
  a static library (C/C++).

* If none of the above are defined then defaults will be used.

All of the mentioned options are mutually exclusive.

Details:

* When WEBVIEW_API is already defined then the linkage is as specified
  by WEBVIEW_API.

* When webview.h is included in C++ code without any options then
  WEBVIEW_API is set to inline to avoid ODR violations. Existing users
  who rely on WEBVIEW_API to default to extern may see undefined
  references unless they define either WEBVIEW_STATIC or WEBVIEW_API
  (as extern or nothing).

* When webview.h is included in C code without any options then
  WEBVIEW_API is set to nothing which defaults to extern as before.

* When WEBVIEW_STATIC is defined then WEBVIEW_API is set to
  nothing (extern).

* When WEBVIEW_BUILD_SHARED is defined then WEBVIEW_API is set to
  __declspec(dllexport) when building for Windows and otherwise
  __attribute__((visibility("default"))).

* When WEBVIEW_SHARED is defined then WEBVIEW_API is set to
  __declspec(dllimport) for Windows and otherwise
  __attribute__((visibility("default"))).
@SteffenL SteffenL added the enhancement New feature, enhancement of an existing feature or a request label Sep 12, 2023
@SteffenL SteffenL added this to the v0.11.0 milestone Sep 12, 2023
@SteffenL SteffenL merged commit dbaf137 into webview:master Sep 16, 2023
27 checks passed
@SteffenL SteffenL deleted the up/easy-api-linkage branch September 16, 2023 13:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, enhancement of an existing feature or a request
Development

Successfully merging this pull request may close these issues.

None yet

1 participant