From 86d219c38847965714df9eba03b07fa2f6e30ecc Mon Sep 17 00:00:00 2001 From: Philip Pronin Date: Mon, 14 Jan 2013 15:03:00 -0800 Subject: [PATCH] fix some of the warning/errors clang 3.1 reports Summary: One step closer to build folly with clang Test Plan: fbconfig -r folly && fbmake opt -j32 to make sure gcc is still able to compile it Reviewed By: tudorb@fb.com FB internal diff: D677716 --- folly/Benchmark.h | 4 +-- folly/Format-inl.h | 12 +++++--- folly/Padded.h | 3 +- folly/Subprocess.cpp | 10 ++---- folly/experimental/Gen-inl.h | 57 +++++++++++++++++++++++------------ folly/test/FormatTest.cpp | 11 +------ folly/test/TimeseriesTest.cpp | 5 +-- 7 files changed, 55 insertions(+), 47 deletions(-) diff --git a/folly/Benchmark.h b/folly/Benchmark.h index 2b68c4fa..5e155f28 100644 --- a/folly/Benchmark.h +++ b/folly/Benchmark.h @@ -1,5 +1,5 @@ /* - * Copyright 2012 Facebook, Inc. + * Copyright 2013 Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -185,7 +185,7 @@ typename std::enable_if< == 2 >::type addBenchmark(const char* file, const char* name, Lambda&& lambda) { - auto execute = [=](unsigned int times) { + auto execute = [=](unsigned int times) -> uint64_t { BenchmarkSuspender::nsSpent = 0; timespec start, end; diff --git a/folly/Format-inl.h b/folly/Format-inl.h index 944e2869..bba7be85 100644 --- a/folly/Format-inl.h +++ b/folly/Format-inl.h @@ -1,5 +1,5 @@ /* - * Copyright 2012 Facebook, Inc. + * Copyright 2013 Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -43,7 +43,9 @@ const size_t kMaxBinaryLength = 8 * sizeof(uintmax_t); template size_t uintToHex(char* buffer, size_t bufLen, Uint v, const char (&repr)[256][2]) { - for (; v >= 256; v >>= 8) { + // 'v >>= 7, v >>= 1' is no more than a work around to get rid of shift size + // warning when Uint = uint8_t (it's false as v >= 256 implies sizeof(v) > 1). + for (; v >= 256; v >>= 7, v >>= 1) { auto b = v & 0xff; bufLen -= 2; buffer[bufLen] = repr[b][0]; @@ -85,7 +87,9 @@ inline size_t uintToHexUpper(char* buffer, size_t bufLen, Uint v) { template size_t uintToOctal(char* buffer, size_t bufLen, Uint v) { auto& repr = formatOctal; - for (; v >= 512; v >>= 9) { + // 'v >>= 7, v >>= 2' is no more than a work around to get rid of shift size + // warning when Uint = uint8_t (it's false as v >= 512 implies sizeof(v) > 1). + for (; v >= 512; v >>= 7, v >>= 2) { auto b = v & 0x1ff; bufLen -= 3; buffer[bufLen] = repr[b][0]; @@ -117,7 +121,7 @@ size_t uintToBinary(char* buffer, size_t bufLen, Uint v) { buffer[--bufLen] = '0'; return bufLen; } - for (; v; v >>= 8) { + for (; v; v >>= 7, v >>= 1) { auto b = v & 0xff; bufLen -= 8; memcpy(buffer + bufLen, &(repr[b][0]), 8); diff --git a/folly/Padded.h b/folly/Padded.h index 4c0af2af..f4d3b9c7 100644 --- a/folly/Padded.h +++ b/folly/Padded.h @@ -1,5 +1,5 @@ /* - * Copyright 2012 Facebook, Inc. + * Copyright 2013 Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,6 +19,7 @@ #include #include +#include #include #include #include diff --git a/folly/Subprocess.cpp b/folly/Subprocess.cpp index e8963dc8..da69e536 100644 --- a/folly/Subprocess.cpp +++ b/folly/Subprocess.cpp @@ -442,12 +442,6 @@ void Subprocess::sendSignal(int signal) { } namespace { -void setNonBlocking(int fd) { - int flags = ::fcntl(fd, F_GETFL); - checkUnixError(flags, "fcntl"); - int r = ::fcntl(fd, F_SETFL, flags | O_NONBLOCK); - checkUnixError(r, "fcntl"); -} std::pair queueFront(const IOBufQueue& queue) { auto* p = queue.front(); @@ -544,7 +538,7 @@ std::pair Subprocess::communicateIOBuf( IOBufQueue data) { std::pair out; - auto readCallback = [&] (int pfd, int cfd) { + auto readCallback = [&] (int pfd, int cfd) -> bool { if (cfd == 1 && flags.readStdout_) { return handleRead(pfd, out.first); } else if (cfd == 2 && flags.readStderr_) { @@ -556,7 +550,7 @@ std::pair Subprocess::communicateIOBuf( } }; - auto writeCallback = [&] (int pfd, int cfd) { + auto writeCallback = [&] (int pfd, int cfd) -> bool { if (cfd == 0 && flags.writeStdin_) { return handleWrite(pfd, data); } else { diff --git a/folly/experimental/Gen-inl.h b/folly/experimental/Gen-inl.h index 6473b254..bde69792 100644 --- a/folly/experimental/Gen-inl.h +++ b/folly/experimental/Gen-inl.h @@ -1,5 +1,5 @@ /* - * Copyright 2012 Facebook, Inc. + * Copyright 2013 Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -175,7 +175,7 @@ class GenImpl : public FBounded { */ template void foreach(Body&& body) const { - this->self().apply([&](Value value) { + this->self().apply([&](Value value) -> bool { body(std::forward(value)); return true; }); @@ -664,7 +664,7 @@ class Until : public Operator> { public GenImpl> { Source source_; Predicate pred_; - public: + public: explicit Generator(Source source, const Predicate& pred) : source_(std::move(source)), pred_(pred) {} @@ -703,7 +703,7 @@ class Until : public Operator> { */ class Take : public Operator { size_t count_; -public: + public: explicit Take(size_t count) : count_(count) {} @@ -756,7 +756,7 @@ public: */ class Skip : public Operator { size_t count_; -public: + public: explicit Skip(size_t count) : count_(count) {} @@ -766,7 +766,7 @@ public: public GenImpl> { Source source_; size_t count_; - public: + public: explicit Generator(Source source, size_t count) : source_(std::move(source)) , count_(count) {} @@ -835,8 +835,8 @@ class Order : public Operator> { Selector selector_; Comparer comparer_; public: - Order(const Selector& selector = Selector(), - const Comparer& comparer = Comparer()) + explicit Order(const Selector& selector = Selector(), + const Comparer& comparer = Comparer()) : selector_(selector) , comparer_(comparer) {} template> { First first_; Second second_; - public: - Composed() {} - Composed(First first, Second second) - : first_(std::move(first)) - , second_(std::move(second)) {} + public: + Composed() {} + Composed(First first, Second second) + : first_(std::move(first)) + , second_(std::move(second)) {} template> { */ class First : public Operator { public: + First() { } + template::type> @@ -1033,6 +1035,8 @@ class First : public Operator { */ class Any : public Operator { public: + Any() { } + template bool compose(const GenImpl& source) const { @@ -1061,7 +1065,7 @@ template class Reduce : public Operator> { Reducer reducer_; public: - Reduce(const Reducer& reducer) + explicit Reduce(const Reducer& reducer) : reducer_(reducer) {} @@ -1093,6 +1097,8 @@ class Reduce : public Operator> { */ class Count : public Operator { public: + Count() { } + template size_t compose(const GenImpl& source) const { @@ -1112,6 +1118,8 @@ class Count : public Operator { */ class Sum : public Operator { public: + Sum() { } + template::type> @@ -1141,8 +1149,8 @@ class Min : public Operator> { Selector selector_; Comparer comparer_; public: - Min(const Selector& selector = Selector(), - const Comparer& comparer = Comparer()) + explicit Min(const Selector& selector = Selector(), + const Comparer& comparer = Comparer()) : selector_(selector) , comparer_(comparer) {} @@ -1211,6 +1219,8 @@ class Append : public Operator> { template class Collect : public Operator> { public: + Collect() { } + template::type> @@ -1243,6 +1253,8 @@ template class Container, template class Allocator> class CollectTemplate : public Operator> { public: + CollectTemplate() { } + template::type, @@ -1255,6 +1267,7 @@ class CollectTemplate : public Operator> { return collection; } }; + /** * Concat - For flattening generators of generators. * @@ -1272,14 +1285,16 @@ class CollectTemplate : public Operator> { * | as(); */ class Concat : public Operator { -public: + public: + Concat() { } + template::type::ValueType> class Generator : public GenImpl> { Source source_; - public: + public: explicit Generator(Source source) : source_(std::move(source)) {} @@ -1326,7 +1341,9 @@ public: * | as(); */ class RangeConcat : public Operator { -public: + public: + RangeConcat() { } + template::RefType> @@ -1348,7 +1365,7 @@ public: template bool apply(Handler&& handler) const { - return source_.apply([&](Range range) { + return source_.apply([&](Range range) -> bool { for (auto& value : range) { if (!handler(value)) { return false; diff --git a/folly/test/FormatTest.cpp b/folly/test/FormatTest.cpp index c34fd49c..96de6bf6 100644 --- a/folly/test/FormatTest.cpp +++ b/folly/test/FormatTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2012 Facebook, Inc. + * Copyright 2013 Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -186,15 +186,6 @@ TEST(Format, Simple) { EXPECT_EQ("42 23 hello worldXX", s); } -namespace { -void testFloat(const char* fmt, double val) { - char buf[100]; - sprintf(buf, to("%", fmt).c_str(), val); - - EXPECT_EQ(buf, fstr(to("{:", fmt, "}"), val)); -} -} // namespace - TEST(Format, Float) { double d = 1; EXPECT_EQ("1", fstr("{}", 1.0)); diff --git a/folly/test/TimeseriesTest.cpp b/folly/test/TimeseriesTest.cpp index 69dbe9ad..7972eda7 100644 --- a/folly/test/TimeseriesTest.cpp +++ b/folly/test/TimeseriesTest.cpp @@ -1,5 +1,5 @@ /* - * Copyright 2012 Facebook, Inc. + * Copyright 2013 Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -13,6 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ + #include "folly/stats/BucketedTimeSeries.h" #include "folly/stats/BucketedTimeSeries-defs.h" @@ -475,7 +476,7 @@ TEST(BucketedTimeSeries, forEachBucket) { vector info; auto fn = [&](const Bucket& bucket, seconds bucketStart, - seconds bucketEnd) { + seconds bucketEnd) -> bool { info.emplace_back(&bucket, bucketStart, bucketEnd); return true; }; -- 2.34.1