Drizzle

Merge lp:~brianaker/drizzle/clang-support into lp:drizzle

  1. clang-support
  2. Merge into 7.2
Proposed by Brian Aker
Status: Merged
Approved by: Brian Aker
Approved revision: no longer in the source branch.
Merged at revision: 2581
Proposed branch: lp:~brianaker/drizzle/clang-support
Merge into: lp:drizzle
Diff against target: 243 lines (+69/-2)
12 files modified
bootstrap.sh (+44/-0)
client/drizzletest.cc (+4/-0)
drizzled/field/datetime.cc (+1/-0)
drizzled/filesort.cc (+1/-0)
drizzled/function/time/unix_timestamp.cc (+1/-0)
drizzled/sql_insert.cc (+3/-0)
drizzled/sql_load.cc (+1/-0)
drizzled/sql_union.cc (+3/-0)
drizzled/table/cache.cc (+1/-0)
drizzled/table_function_container.cc (+1/-0)
plugin/logging_query/logging_query.cc (+1/-0)
plugin/schema_engine/schema.cc (+8/-2)
To merge this branch: bzr merge lp:~brianaker/drizzle/clang-support
Reviewer Review Type Date Requested Status
Drizzle Trunk Pending
Review via email: mp+118033@code.launchpad.net
To post a comment you must log in.
lp:~brianaker/drizzle/clang-support updated
2579. By Drizzle Continuous Integration

added:
  bootstrap.sh
modified:
  client/drizzletest.cc
  drizzled/field/datetime.cc
  drizzled/filesort.cc
  drizzled/function/time/unix_timestamp.cc
  drizzled/sql_insert.cc
  drizzled/sql_load.cc
  drizzled/sql_union.cc
  drizzled/table/cache.cc
  drizzled/table_function_container.cc
  plugin/logging_query/logging_query.cc
  plugin/schema_engine/schema.cc
pending merge tips: (use -v to see all merge revisions)
  Brian Aker 2012-08-02 Add bootstrap script, and fixes for clang.

2580. By Brian Aker

Fixing license on bootsrap.sh file

Revision history for this message
Olaf van der Spek (olafvdspek) wrote :

> (void)(ret);

Is stuff like this really necessary?

Revision history for this message
Mark Atwood (fallenpegasus) wrote :

On Sat, Aug 4, 2012 at 6:31 AM, Olaf van der Spek <email address hidden> wrote:
>> (void)(ret);
>
> Is stuff like this really necessary?

Do have clang compile without warnings, yes it is.

Otherwise the dataflow analyzer in clang determines that the data
result is unused, and issues a warning.

..m

Revision history for this message
Olaf van der Spek (olafvdspek) wrote :

Isn't it used in the assert expression?

Revision history for this message
Mark Atwood (fallenpegasus) wrote :

On Sat, Aug 4, 2012 at 9:39 AM, Olaf van der Spek <email address hidden> wrote:
> Isn't it used in the assert expression?

Expressions in assert() are not expanded or evaluated when NDEBUG is defined.

It is a bug to assume that assert() will evaluate, and it's most
certainly a bug to have an assert() expression that has a side
effect.

A good dataflow analyzer knows that assert() should not count as "data
used". Given that clang's DFA is pretty close to state of the
research art, I would guess that it also knows that.

Back when I was programming in Ada, the compiler I was using could
optimize assertation statements, making them test only occasionally,
only on entrance and exit of the loop they where in, only when
something "interesting" happened in the data flow, and so forth.

..m

Revision history for this message
Olaf van der Spek (olafvdspek) wrote :

I'm aware of assert semantics. But (void)(ret); doesn't generate any code either. Having to use (void) for all data that's only used in assert expressions doesn't seem (entirely) right.

lp:~brianaker/drizzle/clang-support updated
2581. By Brian Aker

Fix type in shell.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'bootstrap.sh'
2--- bootstrap.sh 1970-01-01 00:00:00 +0000
3+++ bootstrap.sh 2012-08-11 15:59:19 +0000
4@@ -0,0 +1,44 @@
5+#!/bin/bash
6+#
7+# Copyright (C) 2012 Brian Aker
8+# All rights reserved.
9+#
10+# Redistribution and use in source and binary forms, with or without
11+# modification, are permitted provided that the following conditions are
12+# met:
13+#
14+# * Redistributions of source code must retain the above copyright
15+# notice, this list of conditions and the following disclaimer.
16+#
17+# * Redistributions in binary form must reproduce the above
18+# copyright notice, this list of conditions and the following disclaimer
19+# in the documentation and/or other materials provided with the
20+# distribution.
21+#
22+# * The names of its contributors may not be used to endorse or
23+# promote products derived from this software without specific prior
24+# written permission.
25+#
26+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
27+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
28+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
29+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
30+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
31+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
32+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
33+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
34+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
35+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
36+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
37+
38+if test -f configure; then make clean; make merge-clean; make distclean; fi;
39+
40+rm -r -f autom4te.cache/ config.h config.log config.status
41+./config/autorun.sh
42+if [ $(uname) = "Darwin" ];
43+then
44+ ./configure CC=clang CXX=clang++ --enable-assert
45+else
46+ ./configure --enable-assert
47+fi
48+make
49
50=== modified file 'client/drizzletest.cc'
51--- client/drizzletest.cc 2012-02-12 06:10:39 +0000
52+++ client/drizzletest.cc 2012-08-11 15:59:19 +0000
53@@ -734,6 +734,10 @@
54 break;
55 }
56 assert(known_arg_type);
57+ if (known_arg_type == false)
58+ {
59+ die("Bad argument");
60+ }
61
62 /* Check required arg */
63 if (arg->ds->length() == 0 && arg->required)
64
65=== modified file 'drizzled/field/datetime.cc'
66--- drizzled/field/datetime.cc 2011-06-20 15:04:56 +0000
67+++ drizzled/field/datetime.cc 2012-08-11 15:59:19 +0000
68@@ -138,6 +138,7 @@
69 size_t tmp_string_len;
70
71 tmp_string_len= temporal.to_string(tmp_string, type::Time::MAX_STRING_LENGTH);
72+ (void)(tmp_string_len);
73 assert(tmp_string_len < type::Time::MAX_STRING_LENGTH);
74 my_error(ER_INVALID_DATETIME_VALUE, MYF(ME_FATALERROR), tmp_string);
75 return 1;
76
77=== modified file 'drizzled/filesort.cc'
78--- drizzled/filesort.cc 2011-10-24 11:55:49 +0000
79+++ drizzled/filesort.cc 2012-08-11 15:59:19 +0000
80@@ -859,6 +859,7 @@
81 from= tmp_buffer;
82 }
83 tmp_length= cs->strnxfrm(to,sort_field->length, (unsigned char*) from, length);
84+ (void)(tmp_length);
85 assert(tmp_length == sort_field->length);
86 }
87 else
88
89=== modified file 'drizzled/function/time/unix_timestamp.cc'
90--- drizzled/function/time/unix_timestamp.cc 2011-04-08 13:16:30 +0000
91+++ drizzled/function/time/unix_timestamp.cc 2012-08-11 15:59:19 +0000
92@@ -70,6 +70,7 @@
93 char buff[DateTime::MAX_STRING_LENGTH];
94 int buff_len;
95 buff_len= temporal.to_string(buff, DateTime::MAX_STRING_LENGTH);
96+ (void)(buff_len);
97 assert((buff_len+1) < DateTime::MAX_STRING_LENGTH);
98 my_error(ER_INVALID_UNIX_TIMESTAMP_VALUE, MYF(0), buff);
99 return 0;
100
101=== modified file 'drizzled/sql_insert.cc'
102--- drizzled/sql_insert.cc 2012-02-12 08:49:06 +0000
103+++ drizzled/sql_insert.cc 2012-08-11 15:59:19 +0000
104@@ -456,6 +456,7 @@
105 if (session->transaction.stmt.hasModifiedNonTransData())
106 session->transaction.all.markModifiedNonTransData();
107 }
108+ (void)(transactional_table);
109 assert(transactional_table || !changed || session->transaction.stmt.hasModifiedNonTransData());
110
111 }
112@@ -1394,6 +1395,8 @@
113
114 changed= (info.copied || info.deleted || info.updated);
115 transactional_table= table->cursor->has_transactions();
116+ (void)(transactional_table);
117+ (void)(changed);
118 assert(transactional_table || !changed ||
119 session->transaction.stmt.hasModifiedNonTransData());
120 table->cursor->ha_release_auto_increment();
121
122=== modified file 'drizzled/sql_load.cc'
123--- drizzled/sql_load.cc 2012-07-11 14:06:00 +0000
124+++ drizzled/sql_load.cc 2012-08-11 15:59:19 +0000
125@@ -412,6 +412,7 @@
126
127 session->my_ok(info.copied + info.deleted, 0, 0L, msg);
128 err:
129+ (void)(transactional_table);
130 assert(transactional_table || !(info.copied || info.deleted) ||
131 session->transaction.stmt.hasModifiedNonTransData());
132 table->cursor->ha_release_auto_increment();
133
134=== modified file 'drizzled/sql_union.cc'
135--- drizzled/sql_union.cc 2011-08-05 13:28:48 +0000
136+++ drizzled/sql_union.cc 2012-08-11 15:59:19 +0000
137@@ -294,7 +294,9 @@
138 information about fields lengths and exact types
139 */
140 if (!is_union_select)
141+ {
142 types= first_sl->item_list;
143+ }
144 else if (sl == first_sl)
145 {
146 /*
147@@ -303,6 +305,7 @@
148 The main reason of this is that we can't create
149 field object without table.
150 */
151+ (void)(empty_table);
152 assert(!empty_table);
153 empty_table= (Table*) session->mem.calloc(sizeof(Table));
154 types.clear();
155
156=== modified file 'drizzled/table/cache.cc'
157--- drizzled/table/cache.cc 2011-07-06 22:48:32 +0000
158+++ drizzled/table/cache.cc 2012-08-11 15:59:19 +0000
159@@ -283,6 +283,7 @@
160 void Cache::insert(table::Concurrent* arg)
161 {
162 CacheMap::iterator returnable= cache.insert(std::make_pair(arg->getShare()->getCacheKey(), arg));
163+ (void)(returnable);
164 assert(returnable != cache.end());
165 }
166
167
168=== modified file 'drizzled/table_function_container.cc'
169--- drizzled/table_function_container.cc 2011-07-05 15:59:49 +0000
170+++ drizzled/table_function_container.cc 2012-08-11 15:59:19 +0000
171@@ -45,6 +45,7 @@
172 void TableFunctionContainer::addFunction(plugin::TableFunction *tool)
173 {
174 std::pair<ToolMap::iterator, bool> ret= table_map.insert(std::make_pair(tool->getPath(), tool));
175+ (void)(ret);
176 assert(ret.second);
177 }
178
179
180=== modified file 'plugin/logging_query/logging_query.cc'
181--- plugin/logging_query/logging_query.cc 2012-06-20 14:17:30 +0000
182+++ plugin/logging_query/logging_query.cc 2012-08-11 15:59:19 +0000
183@@ -374,6 +374,7 @@
184
185 // a single write has a kernel thread lock, thus no need mutex guard this
186 wrv= write(fd, msgbuf.c_str(), msgbuf.length());
187+ (void)(wrv);
188 assert(wrv == msgbuf.length());
189
190 return false;
191
192=== modified file 'plugin/schema_engine/schema.cc'
193--- plugin/schema_engine/schema.cc 2012-07-11 14:06:00 +0000
194+++ plugin/schema_engine/schema.cc 2012-08-11 15:59:19 +0000
195@@ -102,6 +102,7 @@
196 pair<SchemaCache::iterator, bool> ret=
197 schema_cache.insert(make_pair(schema_identifier.getPath(), new message::Schema(schema_message)));
198
199+ (void)(ret);
200 assert(ret.second); // If this has happened, something really bad is going down.
201 }
202 }
203@@ -121,7 +122,9 @@
204 drizzled::message::catalog::shared_ptr message;
205
206 if (not entry->filename.compare(GLOBAL_TEMPORARY_EXT))
207+ {
208 continue;
209+ }
210
211 drizzled::identifier::Catalog identifier(entry->filename);
212
213@@ -176,7 +179,7 @@
214 schema_cache.insert(make_pair(schema_identifier.getPath(), new message::Schema(schema_message)));
215
216 assert(ret.second); // If this has happened, something really bad is going down.
217- return true;
218+ return ret.second;
219 }
220
221 bool Schema::doDropSchema(const identifier::Schema &schema_identifier)
222@@ -222,7 +225,9 @@
223 schema_message.name());
224
225 if (access(schema_identifier.getPath().c_str(), F_OK))
226+ {
227 return false;
228+ }
229
230 if (writeSchemaFile(schema_identifier, schema_message))
231 {
232@@ -233,9 +238,10 @@
233 schema_cache.insert(make_pair(schema_identifier.getPath(), new message::Schema(schema_message)));
234
235 assert(ret.second); // If this has happened, something really bad is going down.
236+ return ret.second;
237 }
238
239- return true;
240+ return false;
241 }
242
243 /**

Subscribers

People subscribed via source and target branches

to all changes: