[libcxx-commits] [PATCH] D130854: [libc++][NFC] Qualify declaval

Sat Aug 6 08:22:02 PDT 2022

Mordante accepted this revision.
Mordante added a comment.
This revision is now accepted and ready to land.

LGTM, but I spotted one more missing `std`.

In D130854#3701255 <https://reviews.llvm.org/D130854#3701255>, @philnik wrote:

> I don't know if this should go into the commit message. It's a bit wordy.
> When removing qualifications and the include from `__type_traits/common_type.h`, running `std/utilities/meta/meta.trans/meta.trans.other/common_type.pass.cpp` results in

Maybe something like

>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:53:66: error: no type named 'type' in 'std::common_type<long, long>'
>       typedef chrono::duration<typename common_type<_Rep1, _Rep2>::type,
>                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__type_traits/common_type.h:107:14: note: in instantiation of template class 'std::common_type<std::chrono::duration<long, std::ratio<3600, 1>>, std::chrono::duration<long, std::ratio<3600, 1>>>' requested here
>       : public common_type<_Tp, _Tp> {};
>                ^
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:279:58: note: in instantiation of template class 'std::common_type<std::chrono::duration<long, std::ratio<3600, 1>>>' requested here
>       _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR typename common_type<duration>::type operator+() const {return typename common_type<duration>::type(*this);}
>                                                            ^
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:308:54: note: in instantiation of template class 'std::chrono::duration<long, std::ratio<3600, 1>>' requested here
>   typedef duration<     int, ratio_multiply<ratio<24>, hours::period>>         days;
>                                                        ^
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:280:81: error: no type named 'type' in 'std::common_type<std::chrono::duration<long, std::ratio<3600, 1>>>'
>       _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR typename common_type<duration>::type operator-() const {return typename common_type<duration>::type(-__rep_);}
>                                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:308:54: note: in instantiation of template class 'std::chrono::duration<long, std::ratio<3600, 1>>' requested here
>   typedef duration<     int, ratio_multiply<ratio<24>, hours::period>>         days;
>                                                        ^
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:53:66: error: no type named 'type' in 'std::common_type<int, int>'
>       typedef chrono::duration<typename common_type<_Rep1, _Rep2>::type,
>                                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__type_traits/common_type.h:107:14: note: in instantiation of template class 'std::common_type<std::chrono::duration<int, std::ratio<86400, 1>>, std::chrono::duration<int, std::ratio<86400, 1>>>' requested here
>       : public common_type<_Tp, _Tp> {};
>                ^
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:279:58: note: in instantiation of template class 'std::common_type<std::chrono::duration<int, std::ratio<86400, 1>>>' requested here
>       _LIBCPP_INLINE_VISIBILITY _LIBCPP_CONSTEXPR typename common_type<duration>::type operator+() const {return typename common_type<duration>::type(*this);}
>                                                            ^
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:309:55: note: in instantiation of template class 'std::chrono::duration<int, std::ratio<86400, 1>>' requested here
>   typedef duration<     int, ratio_multiply<ratio<7>,   days::period>>         weeks;
>                                                         ^
>   19 similar errors omitted
>
> Adding qualifications results in
>
>   While building module 'std' imported from /home/nikolask/llvm-projects/libcxx/libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/common_type.pass.cpp:13:
>   In file included from <module-includes>:17:
>   In file included from /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/math.h:309:
>   In file included from /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/limits:107:
>   In file included from /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/type_traits:432:
>   In file included from /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__type_traits/common_reference.h:13:
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__type_traits/common_type.h:28:43: error: declaration of 'declval' must be imported from module 'std.utility.__utility.declval' before it is required
>   using __cond_type = decltype(false ? std::declval<_Tp>() : std::declval<_Up>());
>                                             ^
>   /home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__utility/declval.h:30:34: note: declaration here is not visible
>   decltype(std::__declval<_Tp>(0)) declval() _NOEXCEPT;
>                                    ^
>   /home/nikolask/llvm-projects/libcxx/libcxx/test/std/utilities/meta/meta.trans/meta.trans.other/common_type.pass.cpp:13:10: fatal error: could not build module 'std'
>   #include <functional>
>    ~~~~~~~~^
>   2 errors generated.

Both have 2 errors and I really prefer the latter... especially since there were 21 instead of 2 before.
This is really a nice improvement for the error messages.

================
Comment at: libcxx/include/__memory/allocator_traits.h:214
 struct __has_max_size<_Alloc, decltype(
     (void)declval<_Alloc&>().max_size()
 )> : true_type { };
----------------

================
Comment at: libcxx/include/__utility/declval.h:30
 template <class _Tp>
-decltype(__declval<_Tp>(0)) declval() _NOEXCEPT;
+decltype(std::__declval<_Tp>(0)) declval() _NOEXCEPT;

----------------
philnik wrote:
> Mordante wrote:
> > This looks off, you only ADL-proved the call to `__declval`. (This isn't needed, but I don't object against this change.)
> Are you asking for something? What else should I have ADL-proofed here?
Nevermind I misread, this `declval` shouldn't be `std::declval`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130854