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

Public identifiers should have a prefix to avoid name clashes #148

Open
ecorm opened this issue Jan 14, 2020 · 6 comments
Open

Public identifiers should have a prefix to avoid name clashes #148

ecorm opened this issue Jan 14, 2020 · 6 comments

Comments

@ecorm
Copy link

ecorm commented Jan 14, 2020

Public identifiers (extern functions and public enumerators) should have a prefix to avoid name collisions with user code and other libraries.

Extern functions should have a prefix such as ryu_.

The ryu_parse Status enumerators should have a prefix such as RYU_ or RYU_STATUS_ to avoid clashing with macros as well as identifiers in the global namespace. The SUCCESS enumerator is particularly susceptible to conflict.

@ecorm
Copy link
Author

ecorm commented Jan 15, 2020

@expnkx Why not? C doesn't have namespaces, so there is no other mechanism to prevent name clashes.

d2s, f2s, and d2exp are very short, and SUCCESS is very common. They are vulnerable to name clashes with user C code and other C libraries.

@ecorm
Copy link
Author

ecorm commented Jan 15, 2020

You can clone that and put it into the namespace. It all depends on you.

That presumes the user of this library is using C++ and not C. It also makes it impossible to use the existing Bazel script to build this library (compiled symbols will not be in the desired namespace).

I don't want to have to fork this library to make it usable in a large project where name clashes are likely.

BTW, shared linking seriously harms the performance and security of computer.

The library can be statically linked, if desired. I doubt inlining the public library functions makes a significant performance difference versus static linking. These aren't small functions we're talking out.

@ulfjack
Copy link
Owner

ulfjack commented Jan 15, 2020

Yeah, they should probably be prefixed. The code is now part of MSVC and (soon?) libc++, and gets prefixed on import there. However, if you want to use it directly, it's a bit annoying that it can cause name conflicts as-is.

@ecorm
Copy link
Author

ecorm commented Jan 15, 2020

My motivation is to use ryu as a substitute for std::from_chars and std::to_chars on compilers that don't support them.

@ecorm
Copy link
Author

ecorm commented Jan 15, 2020

I don't know if the goal of this project is to provide a library for general use, or to serve as a reference for other Ryu implementations (or both). Please let know.

I could hide the ryu symbols within a separately compiled unit, but that would preclude the possibility of making my C++ library header-only. For better or worse, header-only C++ libraries are popular due to their ease of integration. Mine would be optionally header-only, and could be compiled separately if desired.

To avoid breaking the existing API, you could provide alternate headers that have the prefixed versions of functions and other symbols. The prefixed functions would simply delegate to the non-prefixed versions. The extra indirection should be optimized away.

@ulfjack
Copy link
Owner

ulfjack commented Sep 17, 2021

I'm ok with breaking the existing API.

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

No branches or pull requests

3 participants
@ulfjack @ecorm and others