This is an archive of the discontinued LLVM Phabricator instance.

Differential D108940

[libc++][NFC] Rename _LIBCPP_NODISCARD_ATTRIBUTE to _LIBCPP_NODISCARD
ClosedPublic

Authored by ldionne on Aug 30 2021, 1:02 PM.

Details

Reviewers
• Quuxplusone
Mordante
Group Reviewers
Restricted Project
Commits
rG928cad59c783: [libc++][NFC] Rename _LIBCPP_NODISCARD_ATTRIBUTE to _LIBCPP_NODISCARD

Diff Detail

Event Timeline

ldionne requested review of this revision.Aug 30 2021, 1:02 PM
ldionne created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptAug 30 2021, 1:02 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
Harbormaster completed remote builds in B121803: Diff 369528.Aug 30 2021, 1:02 PM
• Quuxplusone added a subscriber: • Quuxplusone.Aug 30 2021, 1:09 PM

LGTM % comment.

libcxx/include/type_traits
2343

This is the single solitary use of _LIBCPP_NODISCARD_ATTRIBUTE in the entire library, and I think it was a mistake. Let's just make this one _LIBCPP_NODISCARD_EXT for consistency. Then we can (continue to) decide what to do about _LIBCPP_NODISCARD_EXT in future PRs.

ldionne updated this revision to Diff 369695.Aug 31 2021, 7:25 AM
ldionne marked an inline comment as done.

Address review comments

ldionne added inline comments.Aug 31 2021, 8:31 AM
libcxx/include/type_traits
2343

Thinking about it further, I don't think we even want to make this nodiscard at all. It's an internal detail of the library anyway and I don't think anyone can mistake it for a modifying operation.

• Quuxplusone accepted this revision as: • Quuxplusone.Aug 31 2021, 9:07 AM
• Quuxplusone added inline comments.
libcxx/include/type_traits
2343

Works for me. Ship it!

Harbormaster completed remote builds in B121929: Diff 369695.Aug 31 2021, 9:14 AM
Mordante accepted this revision.Aug 31 2021, 9:53 AM
Mordante added a subscriber: Mordante.

LGTM!

libcxx/include/type_traits
2343

I've no strong opinion about this.

This revision is now accepted and ready to land.Aug 31 2021, 9:53 AM
Closed by commit rG928cad59c783: [libc++][NFC] Rename _LIBCPP_NODISCARD_ATTRIBUTE to _LIBCPP_NODISCARD (authored by ldionne). · Explain WhyAug 31 2021, 1:06 PM
This revision was automatically updated to reflect the committed changes.
ldionne added a commit: rG928cad59c783: [libc++][NFC] Rename _LIBCPP_NODISCARD_ATTRIBUTE to _LIBCPP_NODISCARD.

Revision Contents

PathSize
libcxx/
include/
12 lines
2 lines

Diff 369528

libcxx/include/__config

Show First 20 Lines • Show All 1,046 Lines • ▼ Show 20 Lines
#endif #endif
#if _LIBCPP_STD_VER > 17 #if _LIBCPP_STD_VER > 17
# define _LIBCPP_CONSTEXPR_AFTER_CXX17 constexpr # define _LIBCPP_CONSTEXPR_AFTER_CXX17 constexpr
#else #else
# define _LIBCPP_CONSTEXPR_AFTER_CXX17 # define _LIBCPP_CONSTEXPR_AFTER_CXX17
#endif #endif
// The _LIBCPP_NODISCARD_ATTRIBUTE should only be used to define other
// NODISCARD macros to the correct attribute.
#if __has_cpp_attribute(nodiscard) || defined(_LIBCPP_COMPILER_MSVC) #if __has_cpp_attribute(nodiscard) || defined(_LIBCPP_COMPILER_MSVC)
# define _LIBCPP_NODISCARD_ATTRIBUTE [[nodiscard]] # define _LIBCPP_NODISCARD [[nodiscard]]
#elif defined(_LIBCPP_COMPILER_CLANG_BASED) && !defined(_LIBCPP_CXX03_LANG) #elif defined(_LIBCPP_COMPILER_CLANG_BASED) && !defined(_LIBCPP_CXX03_LANG)
# define _LIBCPP_NODISCARD_ATTRIBUTE [[clang::warn_unused_result]] # define _LIBCPP_NODISCARD [[clang::warn_unused_result]]
#else #else
// We can't use GCC's [[gnu::warn_unused_result]] and // We can't use GCC's [[gnu::warn_unused_result]] and
// __attribute__((warn_unused_result)), because GCC does not silence them via // __attribute__((warn_unused_result)), because GCC does not silence them via
// (void) cast. // (void) cast.
# define _LIBCPP_NODISCARD_ATTRIBUTE # define _LIBCPP_NODISCARD
#endif #endif
// _LIBCPP_NODISCARD_EXT may be used to apply [[nodiscard]] to entities not // _LIBCPP_NODISCARD_EXT may be used to apply [[nodiscard]] to entities not
// specified as such as an extension. // specified as such as an extension.
#if defined(_LIBCPP_ENABLE_NODISCARD) && !defined(_LIBCPP_DISABLE_NODISCARD_EXT) #if defined(_LIBCPP_ENABLE_NODISCARD) && !defined(_LIBCPP_DISABLE_NODISCARD_EXT)
# define _LIBCPP_NODISCARD_EXT _LIBCPP_NODISCARD_ATTRIBUTE # define _LIBCPP_NODISCARD_EXT _LIBCPP_NODISCARD
#else #else
# define _LIBCPP_NODISCARD_EXT # define _LIBCPP_NODISCARD_EXT
#endif #endif
#if !defined(_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17) && \ #if !defined(_LIBCPP_DISABLE_NODISCARD_AFTER_CXX17) && \
(_LIBCPP_STD_VER > 17 || defined(_LIBCPP_ENABLE_NODISCARD)) (_LIBCPP_STD_VER > 17 || defined(_LIBCPP_ENABLE_NODISCARD))
# define _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_NODISCARD_ATTRIBUTE # define _LIBCPP_NODISCARD_AFTER_CXX17 _LIBCPP_NODISCARD
#else #else
# define _LIBCPP_NODISCARD_AFTER_CXX17 # define _LIBCPP_NODISCARD_AFTER_CXX17
#endif #endif
#if _LIBCPP_STD_VER > 14 && defined(__cpp_inline_variables) && (__cpp_inline_variables >= 201606L) #if _LIBCPP_STD_VER > 14 && defined(__cpp_inline_variables) && (__cpp_inline_variables >= 201606L)
# define _LIBCPP_INLINE_VAR inline # define _LIBCPP_INLINE_VAR inline
#else #else
# define _LIBCPP_INLINE_VAR # define _LIBCPP_INLINE_VAR
▲ Show 20 Lines • Show All 333 Lines • Show Last 20 Lines

libcxx/include/type_traits

Show First 20 Lines • Show All 2,334 Lines • ▼ Show 20 Lines
}; };
#if _LIBCPP_STD_VER > 11 #if _LIBCPP_STD_VER > 11
template <class _Tp> using make_unsigned_t = typename make_unsigned<_Tp>::type; template <class _Tp> using make_unsigned_t = typename make_unsigned<_Tp>::type;
#endif #endif
#ifndef _LIBCPP_CXX03_LANG #ifndef _LIBCPP_CXX03_LANG
template <class _Tp> template <class _Tp>
_LIBCPP_NODISCARD_ATTRIBUTE _LIBCPP_INLINE_VISIBILITY constexpr _LIBCPP_NODISCARD _LIBCPP_INLINE_VISIBILITY constexpr
QuuxplusoneUnsubmitted
Done
ReplyInline Actions

This is the single solitary use of _LIBCPP_NODISCARD_ATTRIBUTE in the entire library, and I think it was a mistake. Let's just make this one _LIBCPP_NODISCARD_EXT for consistency. Then we can (continue to) decide what to do about _LIBCPP_NODISCARD_EXT in future PRs.

Quuxplusone: This is the single solitary use of `_LIBCPP_NODISCARD_ATTRIBUTE` in the entire library, and I…
ldionneAuthorUnsubmitted
Done
ReplyInline Actions

Thinking about it further, I don't think we even want to make this nodiscard at all. It's an internal detail of the library anyway and I don't think anyone can mistake it for a modifying operation.

ldionne: Thinking about it further, I don't think we even want to make this nodiscard at all. It's an…
QuuxplusoneUnsubmitted
Not Done
ReplyInline Actions

Works for me. Ship it!

Quuxplusone: Works for me. Ship it!
MordanteUnsubmitted
Done
ReplyInline Actions

I've no strong opinion about this.

Mordante: I've no strong opinion about this.
typename make_unsigned<_Tp>::type __to_unsigned_like(_Tp __x) noexcept { typename make_unsigned<_Tp>::type __to_unsigned_like(_Tp __x) noexcept {
return static_cast<typename make_unsigned<_Tp>::type>(__x); return static_cast<typename make_unsigned<_Tp>::type>(__x);
} }
#endif #endif
#if _LIBCPP_STD_VER > 14 #if _LIBCPP_STD_VER > 14
template <class...> using void_t = void; template <class...> using void_t = void;
#endif #endif
▲ Show 20 Lines • Show All 2,125 Lines • Show Last 20 Lines