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

nsimd defines i64 etc. in global namespace #84

Open
eschnett opened this issue Feb 22, 2021 · 3 comments
Open

nsimd defines i64 etc. in global namespace #84

eschnett opened this issue Feb 22, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@eschnett
Copy link
Contributor

It seems that nsimd defines types i64 etc. in the global namespace (see file nsimd.h, lines 793 ff.). These should probably be prefixed with nsimd_.

@gquintin gquintin added the question Further information is requested label Feb 22, 2021
@gquintin
Copy link
Contributor

We thought of that during the first drafts of NSIMD. I agree that it is not that good to pollute the global namespace. But when programming in plain C typing nsimd_ each time is not that great and differentiating the C and the C++ APIs (having f32 in plain but not in C++) seems a bad idea too but I am open to suggestions:

for (int i = 0; i < n; i+= vlen(nsimd_f32)) {
  vec(nsimd_f32) va, vb;
  va = vloadu(ptr_a + i, nsimd_f32);
  vb = vloadu(ptr_b + i, nsimd_f32);
  vstore(ptr_dst + i, vadd(va, vb, nsimd_f32));
}

@eschnett
Copy link
Contributor Author

In C++, they could be in a namespace. Using that namespace would then make it easy to access these types.

In C, there could be a separate include file nsimd_types.h with #defines that would remove the suffix. Alternatively, there could be a #define that either enables or disables this behaviour, depending on the default you choose:

#define NSIMD_GLOBAL_TYPES
#include <nsimd/nsimd_all.h>

This behaviour is not causing a problem for me, but it is somewhat at odds with the otherwise very clean design of the library.

@qukhan qukhan added bug Something isn't working and removed question Further information is requested labels Apr 6, 2021
@qukhan
Copy link
Contributor

qukhan commented Apr 6, 2021

This also conflicts with C++17 u8 character literal for UTF-8 strings.

We should either prefix these with nsimd_ or put them in a namespace inlined in nsimd that can be pulled with using. The latter would allow using the nice version when possible (I assume most of the time).

void f() {
  nsimd::types::u8 a; 
  nsimd::u8 b;
  {
    using nsimd::types;
    u8 c;
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants