This is an archive of the discontinued LLVM Phabricator instance.

[libc++][NFC] Qualify declval
ClosedPublic

Authored by philnik on Jul 31 2022, 12:56 PM.

Details

Reviewers
ldionne
Mordante
var-const
Group Reviewers
Restricted Project
Commits
rG73e8e1ba8dad: [libc++][NFC] Qualify declval
Summary

While it's not necessary to qualify calls to declval it makes error messages very crypric if the declaration isn't reachable anymore

For example:

/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

changes with qualification added to:

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.

Diff Detail

Event Timeline

philnik created this revision.Jul 31 2022, 12:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2022, 12:56 PM
philnik requested review of this revision.Jul 31 2022, 12:56 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 31 2022, 12:56 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

Can you update the commit message with a before and after, showing the improvement?
That makes the reason for the change less "cryptic".

SGTM modulo some nits. I really want to have a second look, mainly to make sure I didn't miss some places in this review.

libcxx/include/__iterator/move_iterator.h
56
libcxx/include/__memory/construct_at.h
35
43
libcxx/include/__memory/ranges_construct_at.h
42
libcxx/include/__utility/declval.h
30 ↗(On Diff #448880)

This looks off, you only ADL-proved the call to __declval. (This isn't needed, but I don't object against this change.)

philnik updated this revision to Diff 450177.Aug 4 2022, 4:24 PM
philnik marked 4 inline comments as done.
  • Address comments

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

/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;
                                                      ^
/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<int, std::ratio<86400, 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: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;
                                                      ^
/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<31556952, 1>>, std::chrono::duration<int, std::ratio<31556952, 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<31556952, 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:311:41: note: in instantiation of template class 'std::chrono::duration<int, std::ratio<31556952, 1>>' requested here
typedef duration<     int, ratio_divide<years::period, ratio<12>>>           months;
                                        ^
/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<int, std::ratio<31556952, 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:311:41: note: in instantiation of template class 'std::chrono::duration<int, std::ratio<31556952, 1>>' requested here
typedef duration<     int, ratio_divide<years::period, ratio<12>>>           months;
                                        ^
/home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:53:66: error: no type named 'type' in 'std::common_type<long double, long double>'
    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 double, std::ratio<3600, 1>>, std::chrono::duration<long double, 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 double, 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:542:60: note: in instantiation of template class 'std::chrono::duration<long double, std::ratio<3600, 1>>' requested here
    constexpr chrono::duration<long double, ratio<3600,1>> operator""h(long double __h)
                                                           ^
/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 double, 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:542:60: note: in instantiation of template class 'std::chrono::duration<long double, std::ratio<3600, 1>>' requested here
    constexpr chrono::duration<long double, ratio<3600,1>> operator""h(long double __h)
                                                           ^
/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<60, 1>>, std::chrono::duration<long, std::ratio<60, 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<60, 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:548:31: note: in instantiation of template class 'std::chrono::duration<long, std::ratio<60, 1>>' requested here
    constexpr chrono::minutes operator""min(unsigned long long __m)
                              ^
/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<60, 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:548:31: note: in instantiation of template class 'std::chrono::duration<long, std::ratio<60, 1>>' requested here
    constexpr chrono::minutes operator""min(unsigned long long __m)
                              ^
/home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:53:66: error: no type named 'type' in 'std::common_type<long double, long double>'
    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 double, std::ratio<60, 1>>, std::chrono::duration<long double, std::ratio<60, 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 double, std::ratio<60, 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:553:58: note: in instantiation of template class 'std::chrono::duration<long double, std::ratio<60, 1>>' requested here
    constexpr chrono::duration<long double, ratio<60,1>> operator""min(long double __m)
                                                         ^
/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 double, std::ratio<60, 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:553:58: note: in instantiation of template class 'std::chrono::duration<long double, std::ratio<60, 1>>' requested here
    constexpr chrono::duration<long double, ratio<60,1>> operator""min(long double __m)
                                                         ^
/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, 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 long>, std::chrono::duration<long long>>' 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 long>>' 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:559:31: note: in instantiation of template class 'std::chrono::duration<long long>' requested here
    constexpr chrono::seconds operator""s(unsigned long long __s)
                              ^
/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 long>>'
    _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:559:31: note: in instantiation of template class 'std::chrono::duration<long long>' requested here
    constexpr chrono::seconds operator""s(unsigned long long __s)
                              ^
/home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:53:66: error: no type named 'type' in 'std::common_type<long double, long double>'
    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 double>, std::chrono::duration<long double>>' 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 double>>' 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:564:45: note: in instantiation of template class 'std::chrono::duration<long double>' requested here
    constexpr chrono::duration<long double> operator""s(long double __s)
                                            ^
/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 double>>'
    _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:564:45: note: in instantiation of template class 'std::chrono::duration<long double>' requested here
    constexpr chrono::duration<long double> operator""s(long double __s)
                                            ^
/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, 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 long, std::ratio<1, 1000>>, std::chrono::duration<long long, std::ratio<1, 1000>>>' 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 long, std::ratio<1, 1000>>>' 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:570:36: note: in instantiation of template class 'std::chrono::duration<long long, std::ratio<1, 1000>>' requested here
    constexpr chrono::milliseconds operator""ms(unsigned long long __ms)
                                   ^
/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 long, std::ratio<1, 1000>>>'
    _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:570:36: note: in instantiation of template class 'std::chrono::duration<long long, std::ratio<1, 1000>>' requested here
    constexpr chrono::milliseconds operator""ms(unsigned long long __ms)
                                   ^
/home/nikolask/llvm-projects/libcxx/build/include/c++/v1/__chrono/duration.h:53:66: error: no type named 'type' in 'std::common_type<long double, long double>'
    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 double, std::ratio<1, 1000>>, std::chrono::duration<long double, std::ratio<1, 1000>>>' 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 double, std::ratio<1, 1000>>>' 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:575:52: note: in instantiation of template class 'std::chrono::duration<long double, std::ratio<1, 1000>>' requested here
    constexpr chrono::duration<long double, milli> operator""ms(long double __ms)
                                                   ^
fatal error: too many errors emitted, stopping now [-ferror-limit=]
/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>
 ~~~~~~~~^
21 errors generated.

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.
libcxx/include/__utility/declval.h
30 ↗(On Diff #448880)

Are you asking for something? What else should I have ADL-proofed here?

Mordante accepted this revision.Aug 6 2022, 8:21 AM

LGTM, but I spotted one more missing std.

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.

libcxx/include/__memory/allocator_traits.h
220
libcxx/include/__utility/declval.h
30 ↗(On Diff #448880)

Nevermind I misread, this declval shouldn't be std::declval.

This revision is now accepted and ready to land.Aug 6 2022, 8:21 AM
philnik edited the summary of this revision. (Show Details)Aug 9 2022, 4:39 AM
philnik edited the summary of this revision. (Show Details)
philnik edited the summary of this revision. (Show Details)
philnik updated this revision to Diff 451110.Aug 9 2022, 4:41 AM
philnik marked 3 inline comments as done.
  • Add another one

I noticed declval is misspelled in the commit title, can you update that before landing?

philnik retitled this revision from [libc++][NFC] Qualify declaval to [libc++][NFC] Qualify declval.Aug 10 2022, 8:13 AM
ldionne requested changes to this revision.Aug 11 2022, 10:10 AM

LGTM, but the only problem I have with the patch is that there's currently no way of enforcing it. Can we investigate adding a test for this (maybe with clang-query)? Otherwise, we can just grep our headers with something like grep -E 'declval<' -R libcxx/include | grep -v -E 'std::declval<'. Otherwise, I think it's definitely possible to come up with a single regex to do it, using negative lookahead, but I'm not sure all regex engines support it.

This revision now requires changes to proceed.Aug 11 2022, 10:10 AM

LGTM, but the only problem I have with the patch is that there's currently no way of enforcing it. Can we investigate adding a test for this (maybe with clang-query)? Otherwise, we can just grep our headers with something like grep -E 'declval<' -R libcxx/include | grep -v -E 'std::declval<'. Otherwise, I think it's definitely possible to come up with a single regex to do it, using negative lookahead, but I'm not sure all regex engines support it.

I would prefer to use clang-query over grep, I noticed in the past that grep based tooling evaluates code in comments.

philnik updated this revision to Diff 485111.Dec 23 2022, 7:03 AM

Address comments

ldionne accepted this revision.Jan 12 2023, 8:36 AM
This revision is now accepted and ready to land.Jan 12 2023, 8:36 AM
This revision was landed with ongoing or failed builds.Jan 12 2023, 9:28 AM
This revision was automatically updated to reflect the committed changes.