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

header pollution #38

Open
Xeverous opened this issue Sep 1, 2020 · 6 comments
Open

header pollution #38

Xeverous opened this issue Sep 1, 2020 · 6 comments

Comments

@Xeverous
Copy link

Xeverous commented Sep 1, 2020

span/include/tcb/span.hpp

Lines 133 to 137 in 5d8d366

#ifdef TCB_SPAN_HAVE_STD_BYTE
using byte = std::byte;
#else
using byte = unsigned char;
#endif

The implementation has some usings that put reduced names in enclosing namespace scope. If a user #defines TCB_SPAN_NAMESPACE_NAME to their own, this can have undesired consequences.

A different solution that avoids header pollution - to not define namespace name and use an alias like mylib::span = tcb::span does not really work because there is more than just a span class template, including non-member functions that are rather complex to alias.

I propose to move these usings to some internal namespace, eg TCB_SPAN_NAMESPACE_NAME::detail.

@tcbrindle
Copy link
Owner

Agreed, byte should be in the detail namespace

@Xeverous
Copy link
Author

Xeverous commented Sep 1, 2020

Note if you missed: byte is one among many such aliases.

@tcbrindle
Copy link
Owner

There's contract_violation() and (sometimes) struct contract_violation_error as well as byte... are there any others? size(), data() etc are already in the detail namespace.

@Xeverous
Copy link
Author

Xeverous commented Sep 1, 2020

This is grep -n using result:

133:using byte = std::byte;
135:using byte = unsigned char;
178:using std::data;
179:using std::size;
219:using std::void_t;
222:using void_t = void;
226:using uncvref_t =
257:using remove_pointer_t = typename std::remove_pointer<T>::type;
292:    using storage_type = detail::span_storage<ElementType, Extent>;
296:    using element_type = ElementType;
297:    using value_type = typename std::remove_cv<ElementType>::type;
298:    using size_type = std::size_t;
299:    using difference_type = std::ptrdiff_t;
300:    using pointer = element_type*;
301:    using const_pointer = const element_type*;
302:    using reference = element_type&;
303:    using const_reference = const element_type&;
304:    using iterator = pointer;
305:    using reverse_iterator = std::reverse_iterator<iterator>;
414:    using subspan_return_t =
606:    using type = ElementType;

@tcbrindle
Copy link
Owner

This is grep -n using result:

133:using byte = std::byte;
135:using byte = unsigned char;

Mentioned above.

178:using std::data;
179:using std::size;
219:using std::void_t;
222:using void_t = void;
226:using uncvref_t =
257:using remove_pointer_t = typename std::remove_pointer<T>::type;

These are already in a detail namespace

292:    using storage_type = detail::span_storage<ElementType, Extent>;
296:    using element_type = ElementType;
297:    using value_type = typename std::remove_cv<ElementType>::type;
298:    using size_type = std::size_t;
299:    using difference_type = std::ptrdiff_t;
300:    using pointer = element_type*;
301:    using const_pointer = const element_type*;
302:    using reference = element_type&;
303:    using const_reference = const element_type&;
304:    using iterator = pointer;
305:    using reverse_iterator = std::reverse_iterator<iterator>;
414:    using subspan_return_t =
606:    using type = ElementType;

These are mandated members of span, except for the subspan_return_t helper alias, which should probably be private.

@Xeverous
Copy link
Author

Xeverous commented Sep 1, 2020

Looks like there is nothing more then.

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

2 participants