base/test/trace_event_analyzer.cc - Issue 202993003: Fix "unreachable code" warnings (MSVC warning 4702) in base/.

Unified Diff: base/test/trace_event_analyzer.cc

Issue 202993003: Fix "unreachable code" warnings (MSVC warning 4702) in base/. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/

Patch Set: Created 6 years, 9 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

Index: base/test/trace_event_analyzer.cc

===================================================================

--- base/test/trace_event_analyzer.cc (revision 256983)

+++ base/test/trace_event_analyzer.cc (working copy)

@@ -91,15 +91,15 @@

bool boolean = false;

int int_num = 0;

double double_num = 0.0;

- if (it.value().GetAsString(&str))

+ if (it.value().GetAsString(&str)) {

arg_strings[it.key()] = str;

- else if (it.value().GetAsInteger(&int_num))

+ } else if (it.value().GetAsInteger(&int_num)) {

arg_numbers[it.key()] = static_cast<double>(int_num);

- else if (it.value().GetAsBoolean(&boolean))

+ } else if (it.value().GetAsBoolean(&boolean)) {

arg_numbers[it.key()] = static_cast<double>(boolean ? 1 : 0);

- else if (it.value().GetAsDouble(&double_num))

+ } else if (it.value().GetAsDouble(&double_num)) {

arg_numbers[it.key()] = double_num;

- else {

+ } else {

LOG(WARNING) << "Value type of argument is not supported: " <<

static_cast<int>(it.value().GetType());

continue; // Skip non-supported arguments.

@@ -143,34 +143,30 @@

std::string TraceEvent::GetKnownArgAsString(const std::string& name) const {

std::string arg_string;

- if (GetArgAsString(name, &arg_string))

- return arg_string;

- NOTREACHED();

- return std::string();

+ bool result = GetArgAsString(name, &arg_string);

+ DCHECK(result);

+ return arg_string;

}

double TraceEvent::GetKnownArgAsDouble(const std::string& name) const {

double arg_double;

- if (GetArgAsNumber(name, &arg_double))

- return arg_double;

- NOTREACHED();

- return 0;

+ bool result = GetArgAsNumber(name, &arg_double);

+ DCHECK(result);

+ return arg_double;

}

int TraceEvent::GetKnownArgAsInt(const std::string& name) const {

double arg_double;

- if (GetArgAsNumber(name, &arg_double))

- return static_cast<int>(arg_double);

- NOTREACHED();

- return 0;

+ bool result = GetArgAsNumber(name, &arg_double);

+ DCHECK(result);

+ return static_cast<int>(arg_double);

}

bool TraceEvent::GetKnownArgAsBool(const std::string& name) const {

double arg_double;

- if (GetArgAsNumber(name, &arg_double))

- return (arg_double != 0.0);

- NOTREACHED();

- return false;

+ bool result = GetArgAsNumber(name, &arg_double);

+ DCHECK(result);

+ return (arg_double != 0.0);

}

// QueryNode

@@ -259,9 +255,10 @@

if (is_str)

return !str_value.empty();

- DCHECK(type_ == QUERY_BOOLEAN_OPERATOR)

+ DCHECK_EQ(QUERY_BOOLEAN_OPERATOR, type_)

<< "Invalid query: missing boolean expression";

- DCHECK(left_.get() && (right_.get() || is_unary_operator()));

+ DCHECK(left_.get());

+ DCHECK(right_.get() || is_unary_operator());

if (is_comparison_operator()) {

DCHECK(left().is_value() && right().is_value())

@@ -270,24 +267,24 @@

bool compare_result = false;

if (CompareAsDouble(event, &compare_result))

return compare_result;

- else if (CompareAsString(event, &compare_result))

- return compare_result;

- return false;

+ return CompareAsString(event, &compare_result) ? compare_result : false;

}

// It's a logical operator.

switch (operator_) {

case OP_AND:

return left().Evaluate(event) && right().Evaluate(event);

case OP_OR:

return left().Evaluate(event) || right().Evaluate(event);

case OP_NOT:

return !left().Evaluate(event);

default:

NOTREACHED();

+ return false;

}

- NOTREACHED();

- return false;

}

bool Query::CompareAsDouble(const TraceEvent& event, bool* result) const {

@@ -294,30 +291,36 @@

double lhs, rhs;

if (!left().GetAsDouble(event, &lhs) || !right().GetAsDouble(event, &rhs))

return false;

switch (operator_) {

case OP_EQ:

*result = (lhs == rhs);

return true;

case OP_NE:

*result = (lhs != rhs);

return true;

case OP_LT:

*result = (lhs < rhs);

return true;

case OP_LE:

*result = (lhs <= rhs);

return true;

case OP_GT:

*result = (lhs > rhs);

return true;

case OP_GE:

*result = (lhs >= rhs);

return true;

default:

NOTREACHED();

return false;

}

- return true;

}

bool Query::CompareAsString(const TraceEvent& event, bool* result) const {

@@ -324,6 +327,7 @@

std::string lhs, rhs;

if (!left().GetAsString(event, &lhs) || !right().GetAsString(event, &rhs))

return false;

switch (operator_) {

case OP_EQ:

if (right().is_pattern_)

@@ -333,6 +337,7 @@

else

*result = (lhs == rhs);

return true;

case OP_NE:

if (right().is_pattern_)

*result = !MatchPattern(lhs, rhs);

@@ -341,29 +346,34 @@

else

*result = (lhs != rhs);

return true;

case OP_LT:

*result = (lhs < rhs);

return true;

case OP_LE:

*result = (lhs <= rhs);

return true;

case OP_GT:

*result = (lhs > rhs);

return true;

case OP_GE:

*result = (lhs >= rhs);

return true;

default:

NOTREACHED();

return false;

}

- return true;

}

bool Query::EvaluateArithmeticOperator(const TraceEvent& event,

double* num) const {

- DCHECK(type_ == QUERY_ARITHMETIC_OPERATOR);

- DCHECK(left_.get() && (right_.get() || is_unary_operator()));

+ DCHECK_EQ(QUERY_ARITHMETIC_OPERATOR, type_);

+ DCHECK(left_.get());

+ DCHECK(right_.get() || is_unary_operator());

double lhs = 0, rhs = 0;

if (!left().GetAsDouble(event, &lhs))

@@ -375,22 +385,28 @@

case OP_ADD:

*num = lhs + rhs;

return true;

case OP_SUB:

*num = lhs - rhs;

return true;

case OP_MUL:

*num = lhs * rhs;

return true;

case OP_DIV:

*num = lhs / rhs;

return true;

case OP_MOD:

*num = static_cast<double>(static_cast<int64>(lhs) %

static_cast<int64>(rhs));

return true;

case OP_NEGATE:

*num = -lhs;

return true;

default:

NOTREACHED();

return false;

@@ -401,11 +417,14 @@

switch (type_) {

case QUERY_ARITHMETIC_OPERATOR:

return EvaluateArithmeticOperator(event, num);

case QUERY_EVENT_MEMBER:

return GetMemberValueAsDouble(event, num);

case QUERY_NUMBER:

*num = number_;

return true;

default:

return false;

}

@@ -415,9 +434,11 @@

switch (type_) {

case QUERY_EVENT_MEMBER:

return GetMemberValueAsString(event, str);

case QUERY_STRING:

*str = string_;

return true;

default:

return false;

}

@@ -425,12 +446,12 @@

bool Query::GetMemberValueAsDouble(const TraceEvent& event,

double* num) const {

- DCHECK(type_ == QUERY_EVENT_MEMBER);

+ DCHECK_EQ(QUERY_EVENT_MEMBER, type_);

// This could be a request for a member of |event| or a member of |event|'s

// associated event. Store the target event in the_event:

- const TraceEvent* the_event = (member_ < OTHER_PID) ?

- &event : event.other_event;

+ const TraceEvent* the_event =

+ (member_ < OTHER_PID) ? &event : event.other_event;

// Request for member of associated event, but there is no associated event.

if (!the_event)

@@ -441,38 +462,44 @@

case OTHER_PID:

*num = static_cast<double>(the_event->thread.process_id);

return true;

case EVENT_TID:

case OTHER_TID:

*num = static_cast<double>(the_event->thread.thread_id);

return true;

case EVENT_TIME:

case OTHER_TIME:

*num = the_event->timestamp;

return true;

case EVENT_DURATION:

- if (the_event->has_other_event()) {

- *num = the_event->GetAbsTimeToOtherEvent();

- return true;

- }

- return false;

+ if (!the_event->has_other_event())

+ return false;

+ *num = the_event->GetAbsTimeToOtherEvent();

+ return true;

case EVENT_COMPLETE_DURATION:

- if (the_event->phase == TRACE_EVENT_PHASE_COMPLETE) {

- *num = the_event->duration;

- return true;

- }

- return false;

+ if (the_event->phase != TRACE_EVENT_PHASE_COMPLETE)

+ return false;

+ *num = the_event->duration;

+ return true;

case EVENT_PHASE:

case OTHER_PHASE:

*num = static_cast<double>(the_event->phase);

return true;

case EVENT_HAS_STRING_ARG:

case OTHER_HAS_STRING_ARG:

*num = (the_event->HasStringArg(string_) ? 1.0 : 0.0);

return true;

case EVENT_HAS_NUMBER_ARG:

case OTHER_HAS_NUMBER_ARG:

*num = (the_event->HasNumberArg(string_) ? 1.0 : 0.0);

return true;

case EVENT_ARG:

case OTHER_ARG: {

// Search for the argument name and return its value if found.

@@ -483,10 +510,12 @@

*num = num_i->second;

return true;

}

case EVENT_HAS_OTHER:

// return 1.0 (true) if the other event exists

*num = event.other_event ? 1.0 : 0.0;

return true;

default:

return false;

}

@@ -494,12 +523,12 @@

bool Query::GetMemberValueAsString(const TraceEvent& event,

std::string* str) const {

- DCHECK(type_ == QUERY_EVENT_MEMBER);

+ DCHECK_EQ(QUERY_EVENT_MEMBER, type_);

// This could be a request for a member of |event| or a member of |event|'s

// associated event. Store the target event in the_event:

- const TraceEvent* the_event = (member_ < OTHER_PID) ?

- &event : event.other_event;

+ const TraceEvent* the_event =

+ (member_ < OTHER_PID) ? &event : event.other_event;

// Request for member of associated event, but there is no associated event.

if (!the_event)

@@ -510,14 +539,17 @@

case OTHER_CATEGORY:

*str = the_event->category;

return true;

case EVENT_NAME:

case OTHER_NAME:

*str = the_event->name;

return true;

case EVENT_ID:

case OTHER_ID:

*str = the_event->id;

return true;

case EVENT_ARG:

case OTHER_ARG: {

// Search for the argument name and return its value if found.

@@ -528,6 +560,7 @@

*str = str_i->second;

return true;

}

default:

return false;

}

@@ -686,9 +719,7 @@

// static

TraceAnalyzer* TraceAnalyzer::Create(const std::string& json_events) {

scoped_ptr<TraceAnalyzer> analyzer(new TraceAnalyzer());

- if (analyzer->SetEvents(json_events))

- return analyzer.release();

- return NULL;

+ return analyzer->SetEvents(json_events) ? analyzer.release() : NULL;

}

bool TraceAnalyzer::SetEvents(const std::string& json_events) {

@@ -733,8 +764,8 @@

void TraceAnalyzer::AssociateEvents(const Query& first,

const Query& second,

const Query& match) {

- DCHECK(allow_assocation_changes_) << "AssociateEvents not allowed after "

- "FindEvents";

+ DCHECK(allow_assocation_changes_)

+ << "AssociateEvents not allowed after FindEvents";

// Search for matching begin/end event pairs. When a matching end is found,

// it is associated with the begin event.

@@ -801,16 +832,12 @@

const TraceEvent* TraceAnalyzer::FindFirstOf(const Query& query) {

TraceEventVector output;

- if (FindEvents(query, &output) > 0)

- return output.front();

- return NULL;

+ return (FindEvents(query, &output) > 0) ? output.front() : NULL;

Nico 2014/03/18 10:29:48 no parens

}

const TraceEvent* TraceAnalyzer::FindLastOf(const Query& query) {

TraceEventVector output;

- if (FindEvents(query, &output) > 0)

- return output.back();

- return NULL;

+ return (FindEvents(query, &output) > 0) ? output.back() : NULL;

Nico 2014/03/18 10:29:48 no parens

}

const std::string& TraceAnalyzer::GetThreadName(

@@ -838,7 +865,7 @@

bool GetRateStats(const TraceEventVector& events,

RateStats* stats,

const RateStatsOptions* options) {

- CHECK(stats);

+ DCHECK(stats);

// Need at least 3 events to calculate rate stats.

const size_t kMinEvents = 3;

if (events.size() < kMinEvents) {

@@ -892,9 +919,9 @@

const Query& query,

size_t position,

size_t* return_index) {

- CHECK(return_index);

+ DCHECK(return_index);

for (size_t i = position; i < events.size(); ++i) {

- if (query.Evaluate(*events.at(i))) {

+ if (query.Evaluate(*events[i])) {

Nico 2014/03/18 10:29:48 at() does bounds checking, operator[] doesn't. Is

Peter Kasting 2014/03/18 19:26:18 at() bounds-checks by way of exceptions. We don't

*return_index = i;

return true;

}

@@ -906,18 +933,12 @@

const Query& query,

size_t position,

size_t* return_index) {

- CHECK(return_index);

- if (events.empty())

- return false;

- position = (position < events.size()) ? position : events.size() - 1;

- for (;;) {

- if (query.Evaluate(*events.at(position))) {

- *return_index = position;

+ DCHECK(return_index);

+ for (size_t i = std::min(position + 1, events.size()); i != 0; --i) {

+ if (query.Evaluate(*events[i - 1])) {

+ *return_index = i - 1;

return true;

}

- if (position == 0)

- return false;

- --position;

}

return false;

}

@@ -927,7 +948,7 @@

size_t position,

size_t* return_closest,

size_t* return_second_closest) {

- CHECK(return_closest);

+ DCHECK(return_closest);

if (events.empty() || position >= events.size())

return false;

size_t closest = events.size();

« no previous file with comments | « base/test/expectations/expectation.cc ('k') | base/win/event_trace_consumer_unittest.cc » ('j') | base/win/event_trace_consumer_unittest.cc » ('J')