span icon indicating copy to clipboard operation
span copied to clipboard

header pollution

Open Xeverous opened this issue 5 years ago • 6 comments

https://github.com/tcbrindle/span/blob/5d8d366eca918d0ed3d2d196cbeae6abfd874736/include/tcb/span.hpp#L133-L137

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.

Xeverous avatar Sep 01 '20 12:09 Xeverous

Agreed, byte should be in the detail namespace

tcbrindle avatar Sep 01 '20 13:09 tcbrindle

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

Xeverous avatar Sep 01 '20 15:09 Xeverous

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.

tcbrindle avatar Sep 01 '20 15:09 tcbrindle

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;

Xeverous avatar Sep 01 '20 15:09 Xeverous

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.

tcbrindle avatar Sep 01 '20 15:09 tcbrindle

Looks like there is nothing more then.

Xeverous avatar Sep 01 '20 15:09 Xeverous