Fix "unreachable code" warnings (MSVC warning 4702) in base/.

base/test/trace_event_analyzer.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/test/trace_event_analyzer.h"
 
 #include <algorithm>
 #include <math.h>
 #include <set>
 
@@ skipping 73 matching lines @@
     return false;
   }
 
   // For each argument, copy the type and create a trace_analyzer::TraceValue.
   for (base::DictionaryValue::Iterator it(*args); !it.IsAtEnd();
        it.Advance()) {
     std::string str;
     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.
     }
   }
 
   return true;
 }
 
 double TraceEvent::GetAbsTimeToOtherEvent() const {
@@ skipping 23 matching lines @@
 bool TraceEvent::HasStringArg(const std::string& name) const {
   return (arg_strings.find(name) != arg_strings.end());
 }
 
 bool TraceEvent::HasNumberArg(const std::string& name) const {
   return (arg_numbers.find(name) != arg_numbers.end());
 }
 
 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
 
 QueryNode::QueryNode(const Query& query) : query_(query) {
 }
 
 QueryNode::~QueryNode() {
 }
 
@@ skipping 68 matching lines @@
   bool is_bool = GetAsDouble(event, &bool_value);
   if (is_bool)
     return (bool_value != 0.0);
 
   // string is true if it is non-empty:
   std::string str_value;
   bool is_str = GetAsString(event, &str_value);
   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())
         << "Invalid query: comparison operator used between event member and "
            "value.";
     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 {
   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 {
   std::string lhs, rhs;
   if (!left().GetAsString(event, &lhs) || !right().GetAsString(event, &rhs))
     return false;
+
   switch (operator_) {
     case OP_EQ:
       if (right().is_pattern_)
         *result = MatchPattern(lhs, rhs);
       else if (left().is_pattern_)
         *result = MatchPattern(rhs, lhs);
       else
         *result = (lhs == rhs);
       return true;
+
     case OP_NE:
       if (right().is_pattern_)
         *result = !MatchPattern(lhs, rhs);
       else if (left().is_pattern_)
         *result = !MatchPattern(rhs, lhs);
       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))
     return false;
   if (!is_unary_operator() && !right().GetAsDouble(event, &rhs))
     return false;
 
   switch (operator_) {
     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;
   }
 }
 
 bool Query::GetAsDouble(const TraceEvent& event, double* num) const {
   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;
   }
 }
 
 bool Query::GetAsString(const TraceEvent& event, std::string* str) const {
   switch (type_) {
     case QUERY_EVENT_MEMBER:
       return GetMemberValueAsString(event, str);
+
     case QUERY_STRING:
       *str = string_;
       return true;
+
     default:
       return false;
   }
 }
 
 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)
     return false;
 
   switch (member_) {
     case EVENT_PID:
     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.
       std::map<std::string, double>::const_iterator num_i =
           the_event->arg_numbers.find(string_);
       if (num_i == the_event->arg_numbers.end())
         return false;
       *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;
   }
 }
 
 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)
     return false;
 
   switch (member_) {
     case EVENT_CATEGORY:
     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.
       std::map<std::string, std::string>::const_iterator str_i =
           the_event->arg_strings.find(string_);
       if (str_i == the_event->arg_strings.end())
         return false;
       *str = str_i->second;
       return true;
     }
+
     default:
       return false;
   }
 }
 
 Query::Query(const std::string& str)
     : type_(QUERY_STRING),
       operator_(OP_INVALID),
       member_(EVENT_INVALID),
       number_(0),
@@ skipping 138 matching lines @@
 
 TraceAnalyzer::TraceAnalyzer() : allow_assocation_changes_(true) {
 }
 
 TraceAnalyzer::~TraceAnalyzer() {
 }
 
 // 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) {
   raw_events_.clear();
   if (!ParseEventsFromJson(json_events, &raw_events_))
     return false;
   std::stable_sort(raw_events_.begin(), raw_events_.end());
   ParseMetadata();
   return true;
 }
@@ skipping 24 matching lines @@
   Query match(Query::EventName() == Query::OtherName() &&
               Query::EventCategory() == Query::OtherCategory() &&
               Query::EventId() == Query::OtherId());
 
   AssociateEvents(begin, end, match);
 }
 
 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.
   std::vector<TraceEvent*> begin_stack;
   for (size_t event_index = 0; event_index < raw_events_.size();
        ++event_index) {
 
     TraceEvent& this_event = raw_events_[event_index];
 
     if (second.Evaluate(this_event)) {
@@ skipping 46 matching lines @@
 }
 
 size_t TraceAnalyzer::FindEvents(const Query& query, TraceEventVector* output) {
   allow_assocation_changes_ = false;
   output->clear();
   return FindMatchingEvents(raw_events_, query, output);
 }
 
 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(
     const TraceEvent::ProcessThreadID& thread) {
   // If thread is not found, just add and return empty string.
   return thread_names_[thread];
 }
 
 void TraceAnalyzer::ParseMetadata() {
   for (size_t i = 0; i < raw_events_.size(); ++i) {
     TraceEvent& this_event = raw_events_[i];
     // Check for thread name metadata.
     if (this_event.phase != TRACE_EVENT_PHASE_METADATA ||
         this_event.name != "thread_name")
       continue;
     std::map<std::string, std::string>::const_iterator string_it =
         this_event.arg_strings.find("name");
     if (string_it != this_event.arg_strings.end())
       thread_names_[this_event.thread] = string_it->second;
   }
 }
 
 // TraceEventVector utility functions.
 
 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) {
     LOG(ERROR) << "Not enough events: " << events.size();
     return false;
   }
 
   std::vector<double> deltas;
   size_t num_deltas = events.size() - 1;
   for (size_t i = 0; i < num_deltas; ++i) {
@@ skipping 33 matching lines @@
   stats->standard_deviation_us =
       sqrt(sum_mean_offsets_squared / static_cast<double>(num_deltas - 1));
 
   return true;
 }
 
 bool FindFirstOf(const TraceEventVector& events,
                  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 this change safe?

[Peter Kasting, 2014/03/18 19:26:18]

at() bounds-checks by way of exceptions.  We don't support exceptions, so these
two aren't significantly different; they basically both crash on error.

       *return_index = i;
       return true;
     }
   }
   return false;
 }
 
 bool FindLastOf(const TraceEventVector& events,
                 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;
 }
 
 bool FindClosest(const TraceEventVector& events,
                  const Query& query,
                  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();
   size_t second_closest = events.size();
   for (size_t i = 0; i < events.size(); ++i) {
     if (!query.Evaluate(*events.at(i)))
       continue;
     if (closest == events.size()) {
       closest = i;
       continue;
@@ skipping 27 matching lines @@
   end_position = (end_position < events.size()) ? end_position : events.size();
   size_t count = 0u;
   for (size_t i = begin_position; i < end_position; ++i) {
     if (query.Evaluate(*events.at(i)))
       ++count;
   }
   return count;
 }
 
 }  // namespace trace_analyzer