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

export API #1824

Merged
merged 30 commits into from
Feb 24, 2022
Merged

export API #1824

merged 30 commits into from
Feb 24, 2022

Conversation

boeschf
Copy link
Contributor

@boeschf boeschf commented Feb 1, 2022

In order to export all symbols correctly, I have added one macro per library and one global macro. The content of the macros is determined at configure time depending on build variant (static/shared), compiler, and platforms (linux, mac os) and goes into the library's include directory as export.hpp when installed (at build time it resides at cmake's temporary build directory). The per-library macro is named ARB_LIBNAME_API and goes in front of to-be-exported symbols. The global macro is ARB_SYMBOL_VISIBLE. Below, I sketch out a few examples where these macros would be needed.

In this PR I tried to find all places where any annotation is required. Most of them are in the public headers (and corresponding sources) but some are also added in internal headers, which were required for the unit tests to link properly.

header.hpp

#pragma once 

#include <arbor/export.hpp>

// inline class
// no exports are needed in general (there are exceptions :) )
struct A {
    int foo() { return 42; }
    friend std::ostream& operator<<(std::ostream& o, A const & a) { o << a.foo();  return o; }
};

// template classes are inline
// no exports are needed
template<typename T>
class B {
    T m;
    B(T&& t) : m{std::move(t)} {}
};

// class declaration, implementation in source
// class needs to be exported (note this will make all member symbols visible)
class ARB_ARBOR_API C {
    void bar();
    friend std::ostream& operator<<(std::ostream& o, C const & a);
};

// inline exception class
// even though it is inline, one must make it visible! Otherwise, thrown exceptions
// will cause havoc.
class ARB_SYMBOL_VISIBLE some_error : public std::runtime_error { };

// any class which is wrapped into a std::any/arb::util::any or similar or is put into a variant
// and is part of the public interface (will be referenced outside the library) must have visibiltiy!
class ARB_SYMBOL_VISIBLE D { int a; };

// inline free function
// no need to export
void baz(int i) { std::cout << i << std::endl; }

// free function
// needs to be exported
ARB_ARBOR_API void fuu();

// (extern) global variables
// need to be exported
ARB_ARBOR_API int g;
ARB_ARBOR_API extern int h;

source.cpp

// no export at point of definition needed for member functions
// of an exported class
void C::bar() {
    throw some_error("haha");
}

// friend functions need to be exported though
ARB_ARBOR_API std::ostream& operator<<(std::ostream& o, C const& c) {
    o << 'c';
    return o;
}

// free functions need here an annotation, too!
ARB_ARBOR_API void fuu() {
    std::cout << 42 << std::endl;
}

// need to be annotated, as well
ARB_ARBOR_API int g = 10;
ARB_ARBOR_API int h = 11;

@brenthuisman
Copy link
Contributor

brenthuisman commented Feb 1, 2022

Could you test the cibuildwheel workflow on this change? You can overwrite the ciwheel branch to trigger the action.

[update] This should not matter. Disregard.

@boeschf boeschf marked this pull request as ready for review February 8, 2022 10:52
Copy link
Contributor

@noraabiakar noraabiakar left a comment

Choose a reason for hiding this comment

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

I have some minimal suggestions and questions. Otherwise looks good!

@boeschf
Copy link
Contributor Author

boeschf commented Feb 16, 2022

bors try

bors bot added a commit that referenced this pull request Feb 16, 2022
@bors
Copy link

bors bot commented Feb 17, 2022

try

Timed out.

@boeschf
Copy link
Contributor Author

boeschf commented Feb 21, 2022

bors try

bors bot added a commit that referenced this pull request Feb 21, 2022
@noraabiakar noraabiakar self-assigned this Feb 21, 2022
@bors
Copy link

bors bot commented Feb 21, 2022

try

Timed out.

@noraabiakar
Copy link
Contributor

bors try

bors bot added a commit that referenced this pull request Feb 24, 2022
@bors
Copy link

bors bot commented Feb 24, 2022

try

Build failed:

@noraabiakar noraabiakar merged commit 675fdbc into arbor-sim:master Feb 24, 2022
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