Merge lp:~paul-lucas/zorba/pjl-misc into lp:zorba

Proposed by Paul J. Lucas on 2012-07-26
Status: Merged
Approved by: Matthias Brantner on 2012-07-26
Approved revision: 10955
Merged at revision: 10954
Proposed branch: lp:~paul-lucas/zorba/pjl-misc
Merge into: lp:zorba
Diff against target: 150 lines (+32/-16)
2 files modified
src/util/string/rep_base.h (+7/-3)
src/util/string/rstring.h (+25/-13)
To merge this branch: bzr merge lp:~paul-lucas/zorba/pjl-misc
Reviewer Review Type Date Requested Status
Matthias Brantner 2012-07-26 Approve on 2012-07-26
Paul J. Lucas Approve on 2012-07-26
Review via email: mp+116894@code.launchpad.net

Commit message

1. Tweaked equals().
2. Added "const&" to std_string function arguments.

Description of the change

1. Tweaked equals().
2. Added "const&" to std_string function arguments.

To post a comment you must log in.
Paul J. Lucas (paul-lucas) :
review: Approve
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue job pjl-misc-2012-07-26T15-47-06.784Z is finished. The final status was:

All tests succeeded!

Zorba Build Bot (zorba-buildbot) wrote :

Voting does not meet specified criteria. Required: Approve > 1, Disapprove < 1, Needs Fixing < 1, Pending < 1. Got: 1 Approve.

Shouldn't the following be rewritten for performance:

return s1_n == s2_n && (s1 == s2 || std::memcmp( s1, s2, s1_n ) == 0);

=>

return s1_n == s2_n && (std::memcmp( s1, s2, s1_n ) == 0 ||s1 == s2);

review: Needs Information
Paul J. Lucas (paul-lucas) wrote :

No because || is evaluated left-to-right. If s1 == s2, then there's no point in doing an expensive memcmp().

review: Approve
Zorba Build Bot (zorba-buildbot) wrote :

Validation queue job pjl-misc-2012-07-26T17-07-02.673Z is finished. The final status was:

All tests succeeded!

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/util/string/rep_base.h'
2--- src/util/string/rep_base.h 2012-07-24 08:48:48 +0000
3+++ src/util/string/rep_base.h 2012-07-26 15:45:26 +0000
4@@ -197,8 +197,10 @@
5 }
6
7 /**
8- * Checks whether this representation is shared.
9+ * Checks if the given shared-count indicates whether some string's
10+ * representation is shared.
11 *
12+ * @param count The shared-count to check.
13 * @return Returns \c true only if it is.
14 */
15 static bool is_shared( count_type count ) {
16@@ -206,7 +208,7 @@
17 }
18
19 /**
20- * Checks whether this representation is unsharable.
21+ * Checks whether this representation is sharable.
22 *
23 * @return Returns \c true only if it is.
24 */
25@@ -215,8 +217,10 @@
26 }
27
28 /**
29- * Checks whether this representation is unsharable.
30+ * Checks if the given shared-count indicates whether some string's
31+ * representation is sharable.
32 *
33+ * @param count The shared-count to check.
34 * @return Returns \c true only if it is.
35 */
36 static bool is_sharable( count_type count ) {
37
38=== modified file 'src/util/string/rstring.h'
39--- src/util/string/rstring.h 2012-07-24 08:48:48 +0000
40+++ src/util/string/rstring.h 2012-07-26 15:45:26 +0000
41@@ -2943,7 +2943,7 @@
42 * @return Returns \c true only if \a s1 \c == \a s2.
43 */
44 inline bool equals( char const *s1, size_t s1_n, char const *s2, size_t s2_n ) {
45- return s1_n == s2_n && std::memcmp( s1, s2, s1_n ) == 0;
46+ return s1_n == s2_n && (s1 == s2 || std::memcmp( s1, s2, s1_n ) == 0);
47 }
48
49 template<class Rep> inline bool
50@@ -2957,12 +2957,14 @@
51 }
52
53 template<class Rep> inline bool
54-operator==( rstring<Rep> const &s1, typename rstring<Rep>::std_string s2 ) {
55+operator==( rstring<Rep> const &s1,
56+ typename rstring<Rep>::std_string const &s2 ) {
57 return equals( s1.data(), s1.size(), s2.data(), s2.size() );
58 }
59
60 template<class Rep> inline bool
61-operator==( typename rstring<Rep>::std_string s1, rstring<Rep> const &s2 ) {
62+operator==( typename rstring<Rep>::std_string const &s1,
63+ rstring<Rep> const &s2 ) {
64 return equals( s1.data(), s1.size(), s2.data(), s2.size() );
65 }
66
67@@ -2989,12 +2991,14 @@
68 }
69
70 template<class Rep> inline bool
71-operator!=( rstring<Rep> const &s1, typename rstring<Rep>::std_string s2 ) {
72+operator!=( rstring<Rep> const &s1,
73+ typename rstring<Rep>::std_string const &s2 ) {
74 return !(s1 == s2);
75 }
76
77 template<class Rep> inline bool
78-operator!=( typename rstring<Rep>::std_string s1, rstring<Rep> const &s2 ) {
79+operator!=( typename rstring<Rep>::std_string const &s1,
80+ rstring<Rep> const &s2 ) {
81 return !(s1 == s2);
82 }
83
84@@ -3019,12 +3023,14 @@
85 }
86
87 template<class Rep> inline bool
88-operator<( rstring<Rep> const &s1, typename rstring<Rep>::std_string s2 ) {
89+operator<( rstring<Rep> const &s1,
90+ typename rstring<Rep>::std_string const &s2 ) {
91 return s1.compare( s2 ) < 0;
92 }
93
94 template<class Rep> inline bool
95-operator<( typename rstring<Rep>::std_string s1, rstring<Rep> const &s2 ) {
96+operator<( typename rstring<Rep>::std_string const &s1,
97+ rstring<Rep> const &s2 ) {
98 return s2.compare( s1 ) > 0;
99 }
100
101@@ -3049,12 +3055,14 @@
102 }
103
104 template<class Rep> inline bool
105-operator<=( rstring<Rep> const &s1, typename rstring<Rep>::std_string s2 ) {
106+operator<=( rstring<Rep> const &s1,
107+ typename rstring<Rep>::std_string const &s2 ) {
108 return s1.compare( s2 ) <= 0;
109 }
110
111 template<class Rep> inline bool
112-operator<=( typename rstring<Rep>::std_string s1, rstring<Rep> const &s2 ) {
113+operator<=( typename rstring<Rep>::std_string const &s1,
114+ rstring<Rep> const &s2 ) {
115 return s2.compare( s1 ) >= 0;
116 }
117
118@@ -3079,12 +3087,14 @@
119 }
120
121 template<class Rep> inline bool
122-operator>( rstring<Rep> const &s1, typename rstring<Rep>::std_string s2 ) {
123+operator>( rstring<Rep> const &s1,
124+ typename rstring<Rep>::std_string const &s2 ) {
125 return s1.compare( s2 ) > 0;
126 }
127
128 template<class Rep> inline bool
129-operator>( typename rstring<Rep>::std_string s1, rstring<Rep> const &s2 ) {
130+operator>( typename rstring<Rep>::std_string const &s1,
131+ rstring<Rep> const &s2 ) {
132 return s2.compare( s1 ) < 0;
133 }
134
135@@ -3109,12 +3119,14 @@
136 }
137
138 template<class Rep> inline bool
139-operator>=( rstring<Rep> const &s1, typename rstring<Rep>::std_string s2 ) {
140+operator>=( rstring<Rep> const &s1,
141+ typename rstring<Rep>::std_string const &s2 ) {
142 return s1.compare( s2 ) >= 0;
143 }
144
145 template<class Rep> inline bool
146-operator>=( typename rstring<Rep>::std_string s1, rstring<Rep> const &s2 ) {
147+operator>=( typename rstring<Rep>::std_string const &s1,
148+ rstring<Rep> const &s2 ) {
149 return s2.compare( s1 ) <= 0;
150 }
151

Subscribers

People subscribed via source and target branches