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

[MATLAB] Add C++ ARROW_MATLAB_EXPORT symbol export macro #37228

Closed
kevingurney opened this issue Aug 17, 2023 · 1 comment · Fixed by #37233
Closed

[MATLAB] Add C++ ARROW_MATLAB_EXPORT symbol export macro #37228

kevingurney opened this issue Aug 17, 2023 · 1 comment · Fixed by #37233

Comments

@kevingurney
Copy link
Member

Describe the enhancement requested

Currently, the main shared library used by the MATLAB interface - libarrowproxy - does not explicitly export any symbols.

This hasn't been a major issue so far based on our usage of these symbols - except for in one case - which was being able to define a template specialization of the static make function for NumericArray<T> in a C++ source file. To work around this, we decided to inline these template specializations.

However, this isn't an ideal solution because these functions shouldn't need to be inlined (and, in fact, this could have undesirable performance implications).

In addition, to enable the ability to write C++ unit tests for code in the "Proxy layer", we would need to export symbols from libarrowproxy.

Component(s)

MATLAB

@sgilmore10
Copy link
Member

take

@kevingurney kevingurney added this to the 14.0.0 milestone Aug 17, 2023
kevingurney added a commit that referenced this issue Aug 17, 2023
…37233)

### Rationale for this change

Currently, the main shared library used by the MATLAB interface - `libarrowproxy` - does not explicitly export any symbols.

This hasn't been a major issue so far based on our usage of these symbols - except for in one case - which was being able to define a template specialization of the static make function for `NumericArray<T>` in a C++ source file. To work around this, we decided to inline these template specializations.

However, this isn't an ideal solution because these functions shouldn't need to be inlined (and, in fact, this could have undesirable performance implications).

In addition, to enable the ability to write C++ unit tests for code in the "Proxy layer", we would need to export symbols from `libarrowproxy`.

This is the first in a series of pull requests in which we will export symbols from `libarrowproxy` so we can write C++ unit tests via the GoogleTest framework.

### What changes are included in this PR?

1. Added a C++ `ARROW_MATLAB_EXPORT` macro for exporting symbols in `visibility.h`
2. Added a new header `timestamp_array.h` which declares the template method specialization of the static `make` function for `NumericArray<arrow::TimestampType>`
3. Moved the definition of `NumericArray<arrow::TimestampType>::make` to `timestamp_array.cc`

### Are these changes tested?

New tests points are not needed. These changes are covered by our existing  test suite.

### Are there any user-facing changes?

No.

### Future Directions

1. As mentioned above, this is the first in a series of pull requests. In future PRs we will export more symbols to enable C++ unit testing.
* Closes: #37228

Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this issue Nov 13, 2023
…macro (apache#37233)

### Rationale for this change

Currently, the main shared library used by the MATLAB interface - `libarrowproxy` - does not explicitly export any symbols.

This hasn't been a major issue so far based on our usage of these symbols - except for in one case - which was being able to define a template specialization of the static make function for `NumericArray<T>` in a C++ source file. To work around this, we decided to inline these template specializations.

However, this isn't an ideal solution because these functions shouldn't need to be inlined (and, in fact, this could have undesirable performance implications).

In addition, to enable the ability to write C++ unit tests for code in the "Proxy layer", we would need to export symbols from `libarrowproxy`.

This is the first in a series of pull requests in which we will export symbols from `libarrowproxy` so we can write C++ unit tests via the GoogleTest framework.

### What changes are included in this PR?

1. Added a C++ `ARROW_MATLAB_EXPORT` macro for exporting symbols in `visibility.h`
2. Added a new header `timestamp_array.h` which declares the template method specialization of the static `make` function for `NumericArray<arrow::TimestampType>`
3. Moved the definition of `NumericArray<arrow::TimestampType>::make` to `timestamp_array.cc`

### Are these changes tested?

New tests points are not needed. These changes are covered by our existing  test suite.

### Are there any user-facing changes?

No.

### Future Directions

1. As mentioned above, this is the first in a series of pull requests. In future PRs we will export more symbols to enable C++ unit testing.
* Closes: apache#37228

Lead-authored-by: Sarah Gilmore <sgilmore@mathworks.com>
Co-authored-by: Kevin Gurney <kgurney@mathworks.com>
Signed-off-by: Kevin Gurney <kgurney@mathworks.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants