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

Removed symbols without soname bump #177

Open
ViviCoder opened this issue Nov 11, 2022 · 12 comments
Open

Removed symbols without soname bump #177

ViviCoder opened this issue Nov 11, 2022 · 12 comments
Labels
enhancement New feature or request

Comments

@ViviCoder
Copy link

Between 7.2.0 and 7.3.2, some symbols have been removed from the library:

  • those starting with GB__AemultB_08__, replaced with ones starting with GB__AemultB__;
  • those starting with GB_emult_08_, replaced with ones starting with GB_emult_;
  • GB_flip_binop_code and GB_flip_op (replaced with GB_flip_binop?);
  • those starting with GB_ph, replaced with ones starting with GB_phy.

Removing symbols may cause backward incompatibility, and would normally require a soname bump.
If the symbols above are private, they should not be exported.

@DrTimothyAldenDavis
Copy link
Owner

DrTimothyAldenDavis commented Nov 11, 2022 via email

@ViviCoder
Copy link
Author

I assume you mean they are not supposed to be called from outside the library. Is that right?
Then is there any reason to export those symbols?

@DrTimothyAldenDavis
Copy link
Owner

That's correct. They cannot be called by the user application. They're visible there just for the library itself to find them. The prototypes are not exposed to the user application.

@ViviCoder
Copy link
Author

Please have a look at https://gcc.gnu.org/wiki/Visibility.
It says "C++ visibility support", but it also works for plain C.

@DrTimothyAldenDavis
Copy link
Owner

DrTimothyAldenDavis commented Nov 11, 2022

I have to keep them visible. My MATLAB interface to GraphBLAS relies on them (not all of them, but many).

@ViviCoder
Copy link
Author

By curiosity, why are the user-callable methods not sufficient?

Do you need any of the symbols above? I cannot find any reference to them in the interface, except in GB_rename.h.

If you use private symbols for your interface, why should others not use them as well, for example to create an interface in another language?

@DrTimothyAldenDavis
Copy link
Owner

Good question.

There is a C API Specification that I have to adhere to. I add to that very carefully, using the GxB_* name space. In addition, all objects are opaque, so the user does not have access to the internals of the GrB_Matrix.

For MATLAB, I need to go below the opaque level in a few places. That's safe, because the MATLAB interface is my own, and if I change my matrix formats or internal functions, I can easily update my own MATLAB interface.

I can't do that if I allow others access to the internals. I would be stuck supporting them.

I only use a few of these GB_* methods in my MATLAB interface. My long term goal is to remove those, anyway.

None of the GB_* methods are documented, and none of them are meant to be used externally. I do say that in my GraphBLAS.h include file (line 19). I read the note about external visibility, and that would be a good idea to do. I see that it's compiler specific, which is why I wasn't aware of the options in GCC to do that. I could use that to remove quite a few of my GB_* methods from the externally-visible name space, in the future.

@ViviCoder
Copy link
Author

Thank you for the explanation.

@DrTimothyAldenDavis
Copy link
Owner

let's leave this issue open ... I will try to reduce the # of symbols I expose in the library.

@StefanBruens
Copy link

Maybe do something like Qt does, mark private symbols using symbol versioning. Thus it becomes obvious these are only for internal use, and the matlab bindings are also upgraded when the library is updated.

@DrTimothyAldenDavis
Copy link
Owner

That would be too difficult; there are many many internal symbols in GraphBLAS, many of which are generated automatically.

@StefanBruens
Copy link

Symbol versioning would only apply to exported symbols anyway. Catch the private ones with GB_*, and the remainder with *.

@DrTimothyAldenDavis DrTimothyAldenDavis added the enhancement New feature or request label Jan 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants