[libcxx-commits] [PATCH] D76636: [RFC] Added type trait to remove address space qualifier from type

[John McCall via Phabricator via libcxx-commits]

rjmccall added inline comments.


================
Comment at: libcxx/include/type_traits:713
+template <class _Tp> struct __remove_address_space<_Tp> { typedef _Tp type; };
+template <class _Tp, int I> struct __remove_address_space<_Tp __attribute__((address_space(I)))> { typedef _Tp __attribute__((address_space(I))) type; };
+#endif
----------------
The precedent in C++17 is that you should provide both `__remove_address_space` and `__remove_address_space_t`.

Please make the argument an `unsigned long long`, we don't want to artificially restrict the range here in case the attribute accepts something wider in the future.


================
Comment at: libcxx/test/libcxx/type_traits/address_space.pass.cpp:15
+void foo(){
+  std::__remove_address_space<intAS2> i;
+  (void)i;
----------------
jeroen.dobbelaere wrote:
> Maybe a test that the address space was indeed removed ? (by looking at pointer-compatibility or so ?)
> 
> 
It also needs to be `std::__remove_address_space<intAS2>::type`, but yeah, I think something like:

```
static_assert(!std::is_same<intAS2, int>::value, "address-space qualifier not applied");
static_assert(std::is_same<std::__remove_address_space<intAS2>::type>::value, int>, "address-space qualifier not removed");
static_assert(std::is_same<std::__remove_address_space<const volatile intAS2>::type>::value, const volatile int>, "other qualifiers inappropriately removed");
```

And you should test `__remove_address_space_t` the same way.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76636/new/

https://reviews.llvm.org/D76636