This is an archive of the discontinued LLVM Phabricator instance.

Differential D129503

[libc++] Rename variables to use the snake case instead of camel case
ClosedPublic

Authored by huixie90 on Jul 11 2022, 10:29 AM.

Details

Reviewers
philnik
gulfem
var-const
ldionne
Mordante
Group Reviewers
Restricted Project
Commits
rGe90e7e70ef13: [libc++] Rename variables to use the snake case instead of camel case
Summary

For some reason the pre-commit CI of https://reviews.llvm.org/D129233 was all green so I didn't spot this
https://reviews.llvm.org/B174525

Diff Detail

Event Timeline

huixie90 created this revision.Jul 11 2022, 10:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2022, 10:29 AM
huixie90 requested review of this revision.Jul 11 2022, 10:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2022, 10:29 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript
Herald added a subscriber: libcxx-commits. · View Herald Transcript
huixie90 added reviewers: philnik, gulfem, var-const, ldionne.Jul 11 2022, 10:31 AM
philnik accepted this revision.Jul 11 2022, 10:31 AM

LGTM. I added the check in D129051. You probably just didn't rebase onto it.

This revision is now accepted and ready to land.Jul 11 2022, 10:31 AM
philnik retitled this revision from rename variables to use the snake case instead of camel case to [libc++] Rename variables to use the snake case instead of camel case.Jul 11 2022, 10:32 AM
Mordante accepted this revision.Jul 11 2022, 11:38 AM
Mordante added a subscriber: Mordante.

LGTM.

libcxx/include/__algorithm/set_intersection.h
29

These names look suspicious, the struct has underscores, the members not. Since this patch fixes a CI breakage I prefer to land this patch as is and improve this in a separate patch.

For your information __out is a name you can't use. It's used by Microsoft and will fail in the CI in our nasty-macro test.

Mordante mentioned this in D129233: [libc++][ranges] implement `std::ranges::set_intersection`.Jul 11 2022, 11:40 AM
Harbormaster completed remote builds in B174701: Diff 443684.Jul 11 2022, 11:55 AM
Closed by commit rGe90e7e70ef13: [libc++] Rename variables to use the snake case instead of camel case (authored by huixie90, committed by Mordante). · Explain WhyJul 11 2022, 12:57 PM
This revision was automatically updated to reflect the committed changes.
Mordante added a commit: rGe90e7e70ef13: [libc++] Rename variables to use the snake case instead of camel case.
Mordante added a comment.Jul 11 2022, 12:58 PM

@huixie90 since this fixes the breaking of the ToT I've landed this on your behalf.

Please have a look at the review comment I left earlier.

Mordante mentioned this in D129386: [libc++] Make the naming of private member variables consistent and enforce it through readability-identifier-naming.Jul 12 2022, 11:16 AM

Revision Contents

PathSize
libcxx/
include/
__algorithm/
4 lines

Diff 443728

libcxx/include/__algorithm/set_intersection.h

Show All 20 Lines
#endif #endif
_LIBCPP_BEGIN_NAMESPACE_STD _LIBCPP_BEGIN_NAMESPACE_STD
template <class _InIter1, class _InIter2, class _OutIter> template <class _InIter1, class _InIter2, class _OutIter>
struct __set_intersection_result { struct __set_intersection_result {
_InIter1 in1; _InIter1 in1;
_InIter2 in2; _InIter2 in2;
_OutIter out; _OutIter out;
MordanteUnsubmitted
Not Done
ReplyInline Actions

These names look suspicious, the struct has underscores, the members not. Since this patch fixes a CI breakage I prefer to land this patch as is and improve this in a separate patch.

For your information __out is a name you can't use. It's used by Microsoft and will fail in the CI in our nasty-macro test.

Mordante: These names look suspicious, the struct has underscores, the members not. Since this patch…
// need a constructor as C++03 aggregate init is hard // need a constructor as C++03 aggregate init is hard
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17 _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17
__set_intersection_result(_InIter1&& __inIter1, _InIter2&& __inIter2, _OutIter&& __outIter) __set_intersection_result(_InIter1&& __in_iter1, _InIter2&& __in_iter2, _OutIter&& __out_iter)
: in1(std::move(__inIter1)), in2(std::move(__inIter2)), out(std::move(__outIter)) {} : in1(std::move(__in_iter1)), in2(std::move(__in_iter2)), out(std::move(__out_iter)) {}
}; };
template < class _IterOper, class _Compare, class _InIter1, class _Sent1, class _InIter2, class _Sent2, class _OutIter> template < class _IterOper, class _Compare, class _InIter1, class _Sent1, class _InIter2, class _Sent2, class _OutIter>
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17 __set_intersection_result<_InIter1, _InIter2, _OutIter> _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_AFTER_CXX17 __set_intersection_result<_InIter1, _InIter2, _OutIter>
__set_intersection( __set_intersection(
_InIter1 __first1, _Sent1 __last1, _InIter2 __first2, _Sent2 __last2, _OutIter __result, _Compare&& __comp) { _InIter1 __first1, _Sent1 __last1, _InIter2 __first2, _Sent2 __last2, _OutIter __result, _Compare&& __comp) {
while (__first1 != __last1 && __first2 != __last2) { while (__first1 != __last1 && __first2 != __last2) {
if (__comp(*__first1, *__first2)) if (__comp(*__first1, *__first2))
▲ Show 20 Lines • Show All 57 Lines • Show Last 20 Lines