cjdb added a comment. In D116203#3435729 <https://reviews.llvm.org/D116203#3435729>, @aaron.ballman wrote:
> In D116203#3434761 <https://reviews.llvm.org/D116203#3434761>, @rjmccall > wrote: > >> In D116203#3434515 <https://reviews.llvm.org/D116203#3434515>, @cjdb wrote: >> >>> In D116203#3431612 <https://reviews.llvm.org/D116203#3431612>, @rjmccall >>> wrote: >>> >>>> In D116203#3430332 <https://reviews.llvm.org/D116203#3430332>, >>>> @aaron.ballman wrote: >>>> >>>>> In D116203#3425512 <https://reviews.llvm.org/D116203#3425512>, @cjdb >>>>> wrote: >>>>> >>>>>> I've noticed that libstdc++ has `using __remove_cv = typename >>>>>> remove_cv<T>::type`, which causes Clang to chuck a wobbly. Changing from >>>>>> `KEYWORD` to `TYPE_TRAIT_1` didn't seem to fix anything. >>>>>> Is there a way we can work around this, or should we just rename >>>>>> `__remove_cv` and friends to something else? >>>>> >>>>> You could work around it by taking note that you're in a libstdc++ system >>>>> header and do a special dance, but because these are in the >>>>> implementation's namespace, I think it's probably kinder for everyone to >>>>> just pick a different name. >>> >>> I was hoping we could do something similar to `struct __remove_cv` which >>> would issue a warning? >>> >>>>> If you wanted to be especially mean, you could go with `__remove_cvr`, >>>>> but I'd suggest `__remove_cv_qualifiers` instead. However, what about >>>>> `restrict` qualifiers? We support them in C++: >>>>> https://godbolt.org/z/11EPefhjf >>>> >>>> Along with a fair number of other vendor qualifiers, yeah. I think you >>>> have to make a policy decision about whether the intent of >>>> `std::remove_cv` is really to just remove CV qualifiers or to produce an >>>> unqualified type (at the outermost level). The former is probably more >>>> defensible, even if it makes the transform less useful in the presence of >>>> extended qualifiers. >>> >>> I'm partial to `std::remove_cv` being faithful to its name, unless existing >>> implementations do something else already. I don't mind adding support for >>> the other stuff, but if there's more than just add/remove `restrict`, we're >>> going to have a combinatorial explosion for removes. Is there an alternate >>> way we can approach this? >>> Possibly: >>> >>> template<class T> >>> using remove_const_t = __remove_qualifiers(T, const); >>> >>> >>> template<class T> >>> using remove_reference_t = __remove_qualifiers(T, &, &&); >>> >>> template<class T> >>> using remove_rcvref_t = __remove_qualifiers(T, const, volatile, restrict, >>> &, &&); // rcv instead of cvr to prevent a typo with remove_cvref_t >> >> I don't think it's worth adding that parsing complexity for a builtin that >> we expect to only be used in system headers. Let's just remove `const` and >> `volatile` and leave other qualifiers in place. > > I come down on the opposite side of the fence and think it should remove all > qualifiers (or that should be an interface we support in addition to removing > just cv qualifiers). WG14 adopted > https://www9.open-std.org/jtc1/sc22/wg14/www/docs/n2927.htm into C23 with a > `remove_quals` function which removes *all* qualifiers (including `_Atomic`), > as an example. But even in C++ mode, I fail to see why we wouldn't want > <some> interface for stripping `__restrict` the same as `const` or `volatile` > along with some interface for stripping all qualifiers period (I don't see a > huge need for a `__remove_cvr` if we have the ability to remove all > qualifiers, so I don't think we need the combinatorial explosion or a complex > interface). Basically -- if we have extensions like `__restrict` or _Atomic, > we should fully support them rather than halfway do it. Having `__remove_cv` do more than it's advertised to do doesn't sound like a great idea to me. Both libc++ <https://github.com/llvm/llvm-project/blob/main/libcxx/include/type_traits#L647> and libstdc++ <https://github.com/gcc-mirror/gcc/blob/master/libstdc%2B%2B-v3/include/std/type_traits#L1561> define `std::remove_cv` without extensions, and I think it would be surprising for `__remove_cv` to remove anything else. I'm not against adding `__remove_restrict`, `__remove_qualifiers` (although "qualifier" could imply ref-qualifiers too?), etc.. I suppose that in the rare case someone wants to remove `volatile restrict` and keep `const&`, it's possible to do `__add_const(__remove_qualifiers(T)&)`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116203/new/ https://reviews.llvm.org/D116203 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits