Commit d8283428 authored by pkasting@chromium.org's avatar pkasting@chromium.org

Fixes for re-enabling more MSVC level 4 warnings: mojo/ edition

This contains fixes for the following sorts of issues:
* Assignment inside conditional
* Signedness mismatch

BUG=81439
TEST=none

Review URL: https://codereview.chromium.org/372163005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@281964 0039d316-1c4b-4281-b951-d872f2087c98
parent aeed3f4c
...@@ -19,6 +19,16 @@ ...@@ -19,6 +19,16 @@
namespace mojo { namespace mojo {
namespace { namespace {
class PtrToMemberHelper {
public:
int member;
};
bool DcheckTestHelper(bool* was_called) {
*was_called = true;
return false;
}
class LoggingTest : public testing::Test { class LoggingTest : public testing::Test {
public: public:
LoggingTest() : environment_(NULL, &kMockLogger) { LoggingTest() : environment_(NULL, &kMockLogger) {
...@@ -169,28 +179,28 @@ TEST_F(LoggingTest, LogStream) { ...@@ -169,28 +179,28 @@ TEST_F(LoggingTest, LogStream) {
MOJO_LOG_STREAM(INFO) << "hello"; MOJO_LOG_STREAM(INFO) << "hello";
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-3, "hello"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 3, "hello"), last_message());
ResetMockLogger(); ResetMockLogger();
MOJO_LOG_STREAM(ERROR) << "hi " << 123; MOJO_LOG_STREAM(ERROR) << "hi " << 123;
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-3, "hi 123"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 3, "hi 123"), last_message());
} }
TEST_F(LoggingTest, LazyLogStream) { TEST_F(LoggingTest, LazyLogStream) {
MOJO_LAZY_LOG_STREAM(INFO, true) << "hello"; MOJO_LAZY_LOG_STREAM(INFO, true) << "hello";
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-3, "hello"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 3, "hello"), last_message());
ResetMockLogger(); ResetMockLogger();
MOJO_LAZY_LOG_STREAM(ERROR, true) << "hi " << 123; MOJO_LAZY_LOG_STREAM(ERROR, true) << "hi " << 123;
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-3, "hi 123"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 3, "hi 123"), last_message());
ResetMockLogger(); ResetMockLogger();
...@@ -204,15 +214,19 @@ TEST_F(LoggingTest, LazyLogStream) { ...@@ -204,15 +214,19 @@ TEST_F(LoggingTest, LazyLogStream) {
ResetMockLogger(); ResetMockLogger();
bool x = false; PtrToMemberHelper helper;
helper.member = 1;
int PtrToMemberHelper::*member_ptr = &PtrToMemberHelper::member;
// This probably fails to compile if we forget to parenthesize the condition // This probably fails to compile if we forget to parenthesize the condition
// in the macro (= has low precedence, and needs an lvalue on the LHS). // in the macro (.* has lower precedence than !, which can't apply to
MOJO_LAZY_LOG_STREAM(ERROR, x = true) << "hello"; // |helper|).
MOJO_LAZY_LOG_STREAM(ERROR, helper.*member_ptr == 1) << "hello";
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
ResetMockLogger(); ResetMockLogger();
MOJO_LAZY_LOG_STREAM(WARNING, x = false) << "hello"; MOJO_LAZY_LOG_STREAM(WARNING, helper.*member_ptr == 0) << "hello";
EXPECT_FALSE(log_message_was_called()); EXPECT_FALSE(log_message_was_called());
} }
...@@ -249,14 +263,14 @@ TEST_F(LoggingTest, Log) { ...@@ -249,14 +263,14 @@ TEST_F(LoggingTest, Log) {
MOJO_LOG(INFO) << "hello"; MOJO_LOG(INFO) << "hello";
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-3, "hello"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 3, "hello"), last_message());
ResetMockLogger(); ResetMockLogger();
MOJO_LOG(ERROR) << "hello"; MOJO_LOG(ERROR) << "hello";
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-3, "hello"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 3, "hello"), last_message());
ResetMockLogger(); ResetMockLogger();
...@@ -275,7 +289,7 @@ TEST_F(LoggingTest, Log) { ...@@ -275,7 +289,7 @@ TEST_F(LoggingTest, Log) {
MOJO_LOG(ERROR) << "hello"; MOJO_LOG(ERROR) << "hello";
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-3, "hello"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 3, "hello"), last_message());
} }
TEST_F(LoggingTest, LogIf) { TEST_F(LoggingTest, LogIf) {
...@@ -289,43 +303,33 @@ TEST_F(LoggingTest, LogIf) { ...@@ -289,43 +303,33 @@ TEST_F(LoggingTest, LogIf) {
EXPECT_FALSE(log_message_was_called()); EXPECT_FALSE(log_message_was_called());
ResetMockLogger(); ResetMockLogger();
Environment::GetDefaultLogger()->SetMinimumLogLevel(MOJO_LOG_LEVEL_ERROR);
bool x = false; bool x = true;
// Also try to make sure that we parenthesize the condition properly. // Also try to make sure that we parenthesize the condition properly.
MOJO_LOG_IF(INFO, x = true) << "hello"; MOJO_LOG_IF(INFO, false || x) << "hello";
EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-3, "hello"), last_message());
ResetMockLogger();
MOJO_LOG_IF(INFO, x = false) << "hello";
EXPECT_FALSE(log_message_was_called()); EXPECT_FALSE(log_message_was_called());
ResetMockLogger(); ResetMockLogger();
Environment::GetDefaultLogger()->SetMinimumLogLevel(MOJO_LOG_LEVEL_ERROR);
ResetMockLogger();
MOJO_LOG_IF(INFO, 0 != 1) << "hello"; MOJO_LOG_IF(INFO, 0 != 1) << "hello";
EXPECT_FALSE(log_message_was_called()); EXPECT_FALSE(log_message_was_called());
ResetMockLogger(); ResetMockLogger();
MOJO_LOG_IF(WARNING, 1+1 == 2) << "hello"; MOJO_LOG_IF(WARNING, 1 + 1 == 2) << "hello";
EXPECT_FALSE(log_message_was_called()); EXPECT_FALSE(log_message_was_called());
ResetMockLogger(); ResetMockLogger();
MOJO_LOG_IF(ERROR, 1*2 == 2) << "hello"; MOJO_LOG_IF(ERROR, 1 * 2 == 2) << "hello";
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_ERROR, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-3, "hello"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 3, "hello"), last_message());
ResetMockLogger(); ResetMockLogger();
MOJO_LOG_IF(FATAL, 1*2 == 3) << "hello"; MOJO_LOG_IF(FATAL, 1 * 2 == 3) << "hello";
EXPECT_FALSE(log_message_was_called()); EXPECT_FALSE(log_message_was_called());
ResetMockLogger(); ResetMockLogger();
...@@ -342,21 +346,25 @@ TEST_F(LoggingTest, Check) { ...@@ -342,21 +346,25 @@ TEST_F(LoggingTest, Check) {
ResetMockLogger(); ResetMockLogger();
bool x = true; PtrToMemberHelper helper;
helper.member = 0;
int PtrToMemberHelper::*member_ptr = &PtrToMemberHelper::member;
// Also try to make sure that we parenthesize the condition properly. // Also try to make sure that we parenthesize the condition properly.
MOJO_CHECK(x = false) << "hello"; MOJO_CHECK(helper.*member_ptr == 1) << "hello";
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_FATAL, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_FATAL, last_log_level());
// Different compilers have different ideas about the line number of a split // Different compilers have different ideas about the line number of a split
// line. // line.
int line = __LINE__; int line = __LINE__;
EXPECT_EQ(ExpectedLogMessage(line-5, "Check failed: x = false. hello"), EXPECT_EQ(ExpectedLogMessage(line - 5,
"Check failed: helper.*member_ptr == 1. hello"),
last_message()); last_message());
ResetMockLogger(); ResetMockLogger();
// Also test a "naked" |MOJO_CHECK()|s. // Also test a "naked" |MOJO_CHECK()|s.
MOJO_CHECK(1+2 == 3); MOJO_CHECK(1 + 2 == 3);
EXPECT_FALSE(log_message_was_called()); EXPECT_FALSE(log_message_was_called());
} }
...@@ -373,7 +381,7 @@ TEST_F(LoggingTest, Dlog) { ...@@ -373,7 +381,7 @@ TEST_F(LoggingTest, Dlog) {
#else #else
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-6, "hello"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 6, "hello"), last_message());
#endif #endif
} }
...@@ -396,7 +404,7 @@ TEST_F(LoggingTest, DlogIf) { ...@@ -396,7 +404,7 @@ TEST_F(LoggingTest, DlogIf) {
#else #else
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_INFO, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-6, "hello"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 6, "hello"), last_message());
#endif #endif
ResetMockLogger(); ResetMockLogger();
...@@ -411,7 +419,7 @@ TEST_F(LoggingTest, DlogIf) { ...@@ -411,7 +419,7 @@ TEST_F(LoggingTest, DlogIf) {
#else #else
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_WARNING, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_WARNING, last_log_level());
EXPECT_EQ(ExpectedLogMessage(__LINE__-6, "hello"), last_message()); EXPECT_EQ(ExpectedLogMessage(__LINE__ - 6, "hello"), last_message());
#endif #endif
} }
...@@ -429,19 +437,30 @@ TEST_F(LoggingTest, Dcheck) { ...@@ -429,19 +437,30 @@ TEST_F(LoggingTest, Dcheck) {
// |MOJO_DCHECK()| should compile (but not evaluate) its condition even for // |MOJO_DCHECK()| should compile (but not evaluate) its condition even for
// non-debug builds. (Hopefully, we'll get an unused variable error if it // non-debug builds. (Hopefully, we'll get an unused variable error if it
// fails to compile the condition.) // fails to compile the condition.)
bool x = true; bool was_called = false;
MOJO_DCHECK(x = false) << "hello"; MOJO_DCHECK(DcheckTestHelper(&was_called)) << "hello";
#ifdef NDEBUG #ifdef NDEBUG
EXPECT_FALSE(was_called);
EXPECT_FALSE(log_message_was_called()); EXPECT_FALSE(log_message_was_called());
#else #else
EXPECT_TRUE(was_called);
EXPECT_TRUE(log_message_was_called()); EXPECT_TRUE(log_message_was_called());
EXPECT_EQ(MOJO_LOG_LEVEL_FATAL, last_log_level()); EXPECT_EQ(MOJO_LOG_LEVEL_FATAL, last_log_level());
// Different compilers have different ideas about the line number of a split // Different compilers have different ideas about the line number of a split
// line. // line.
int line = __LINE__; int line = __LINE__;
EXPECT_EQ(ExpectedLogMessage(line-8, "Check failed: x = false. hello"), EXPECT_EQ(
last_message()); ExpectedLogMessage(line - 10,
"Check failed: DcheckTestHelper(&was_called). hello"),
last_message());
#endif #endif
ResetMockLogger();
// Also try to make sure that we parenthesize the condition properly.
bool x = true;
MOJO_DCHECK(false || x) << "hello";
EXPECT_FALSE(log_message_was_called());
} }
} // namespace } // namespace
......
...@@ -354,7 +354,7 @@ void Node::Embed(const String& url) { ...@@ -354,7 +354,7 @@ void Node::Embed(const String& url) {
Node::Node() Node::Node()
: manager_(NULL), : manager_(NULL),
id_(-1), id_(static_cast<Id>(-1)),
parent_(NULL), parent_(NULL),
active_view_(NULL) {} active_view_(NULL) {}
......
...@@ -75,7 +75,7 @@ View::View(ViewManager* manager) ...@@ -75,7 +75,7 @@ View::View(ViewManager* manager)
manager_(manager) {} manager_(manager) {}
View::View() View::View()
: id_(-1), : id_(static_cast<Id>(-1)),
node_(NULL), node_(NULL),
manager_(NULL) {} manager_(NULL) {}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment