Modernize and improve parse_tree.cc - Modernize with = default, std::make_unique(). - Use early returns, remove else after returns, and use ternary operators. - Skip the loop earlier in ListNode::SortList(). Change-Id: I4211b1a496a00359d3730958bfc1c85957f92c9a Reviewed-on: https://gn-review.googlesource.com/c/gn/+/19321 Reviewed-by: Takuto Ikuta <tikuta@google.com> Commit-Queue: Takuto Ikuta <tikuta@google.com>
diff --git a/src/gn/parse_tree.cc b/src/gn/parse_tree.cc index 34602b1..feccdb2 100644 --- a/src/gn/parse_tree.cc +++ b/src/gn/parse_tree.cc
@@ -26,6 +26,9 @@ const char kJsonBeforeComment[] = "before_comment"; const char kJsonSuffixComment[] = "suffix_comment"; const char kJsonAfterComment[] = "after_comment"; + +namespace { + const char kJsonLocation[] = "location"; const char kJsonLocationBeginLine[] = "begin_line"; const char kJsonLocationBeginColumn[] = "begin_column"; @@ -36,8 +39,6 @@ const char kJsonBeginToken[] = "begin_token"; const char kJsonEnd[] = "end"; -namespace { - enum DepsCategory { DEPS_CATEGORY_LOCAL, DEPS_CATEGORY_RELATIVE, @@ -231,25 +232,27 @@ } void ParseNode::AddCommentsJSONNodes(base::Value* out_value) const { - if (comments_) { - if (comments_->before().size()) { - base::Value comment_values(base::Value::Type::LIST); - for (const auto& token : comments_->before()) - comment_values.GetList().push_back(base::Value(token.value())); - out_value->SetKey(kJsonBeforeComment, std::move(comment_values)); - } - if (comments_->suffix().size()) { - base::Value comment_values(base::Value::Type::LIST); - for (const auto& token : comments_->suffix()) - comment_values.GetList().push_back(base::Value(token.value())); - out_value->SetKey(kJsonSuffixComment, std::move(comment_values)); - } - if (comments_->after().size()) { - base::Value comment_values(base::Value::Type::LIST); - for (const auto& token : comments_->after()) - comment_values.GetList().push_back(base::Value(token.value())); - out_value->SetKey(kJsonAfterComment, std::move(comment_values)); - } + if (!comments_) { + return; + } + + if (comments_->before().size()) { + base::Value comment_values(base::Value::Type::LIST); + for (const auto& token : comments_->before()) + comment_values.GetList().push_back(base::Value(token.value())); + out_value->SetKey(kJsonBeforeComment, std::move(comment_values)); + } + if (comments_->suffix().size()) { + base::Value comment_values(base::Value::Type::LIST); + for (const auto& token : comments_->suffix()) + comment_values.GetList().push_back(base::Value(token.value())); + out_value->SetKey(kJsonSuffixComment, std::move(comment_values)); + } + if (comments_->after().size()) { + base::Value comment_values(base::Value::Type::LIST); + for (const auto& token : comments_->after()) + comment_values.GetList().push_back(base::Value(token.value())); + out_value->SetKey(kJsonAfterComment, std::move(comment_values)); } } @@ -279,7 +282,7 @@ #undef RETURN_IF_MATCHES_NAME NOTREACHED() << str_type; - return std::unique_ptr<ParseNode>(); + return nullptr; } // AccessorNode --------------------------------------------------------------- @@ -295,7 +298,7 @@ Value AccessorNode::Execute(Scope* scope, Err* err) const { if (subscript_) return ExecuteSubscriptAccess(scope, err); - else if (member_) + if (member_) return ExecuteScopeAccess(scope, err); NOTREACHED(); return Value(); @@ -304,7 +307,7 @@ LocationRange AccessorNode::GetRange() const { if (subscript_) return LocationRange(base_.location(), subscript_->GetRange().end()); - else if (member_) + if (member_) return LocationRange(base_.location(), member_->GetRange().end()); NOTREACHED(); return LocationRange(); @@ -361,14 +364,15 @@ } if (base_value->type() == Value::LIST) { return ExecuteArrayAccess(scope, base_value, err); - } else if (base_value->type() == Value::SCOPE) { - return ExecuteScopeSubscriptAccess(scope, base_value, err); - } else { - *err = MakeErrorDescribing( - std::string("Expecting either a list or a scope for subscript, got ") + - Value::DescribeType(base_value->type()) + "."); - return Value(); } + if (base_value->type() == Value::SCOPE) { + return ExecuteScopeSubscriptAccess(scope, base_value, err); + } + + *err = MakeErrorDescribing( + std::string("Expecting either a list or a scope for subscript, got ") + + Value::DescribeType(base_value->type()) + "."); + return Value(); } Value AccessorNode::ExecuteArrayAccess(Scope* scope, @@ -376,8 +380,9 @@ Err* err) const { size_t index = 0; if (!ComputeAndValidateListIndex(scope, base_value->list_value().size(), - &index, err)) + &index, err)) { return Value(); + } return base_value->list_value()[index]; } @@ -593,7 +598,8 @@ if (begin_token_.type() != Token::INVALID && end_->value().type() != Token::INVALID) { return begin_token_.range().Union(end_->value().range()); - } else if (!statements_.empty()) { + } + if (!statements_.empty()) { return statements_[0]->GetRange().Union( statements_[statements_.size() - 1]->GetRange()); } @@ -634,9 +640,9 @@ std::unique_ptr<BlockNode> ret; if (result_mode == kDumpResultModeReturnsScope) { - ret.reset(new BlockNode(BlockNode::RETURNS_SCOPE)); + ret = std::make_unique<BlockNode>(BlockNode::RETURNS_SCOPE); } else if (result_mode == kDumpResultModeDiscardsResult) { - ret.reset(new BlockNode(BlockNode::DISCARDS_RESULT)); + ret = std::make_unique<BlockNode>(BlockNode::DISCARDS_RESULT); } else { NOTREACHED(); } @@ -691,9 +697,8 @@ } LocationRange ConditionNode::GetRange() const { - if (if_false_) - return if_token_.range().Union(if_false_->GetRange()); - return if_token_.range().Union(if_true_->GetRange()); + return if_token_.range().Union(if_false_ ? if_false_->GetRange() + : if_true_->GetRange()); } Err ConditionNode::MakeErrorDescribing(const std::string& msg, @@ -749,9 +754,8 @@ LocationRange FunctionCallNode::GetRange() const { if (function_.type() == Token::INVALID) return LocationRange(); // This will be null in some tests. - if (block_) - return function_.range().Union(block_->GetRange()); - return function_.range().Union(args_->GetRange()); + return function_.range().Union(block_ ? block_->GetRange() + : args_->GetRange()); } Err FunctionCallNode::MakeErrorDescribing(const std::string& msg, @@ -859,7 +863,7 @@ // ListNode ------------------------------------------------------------------- -ListNode::ListNode() {} +ListNode::ListNode() = default; ListNode::~ListNode() = default; @@ -938,11 +942,13 @@ const ParseNode* node = contents_[i].get(); if (!node->AsLiteral() && !node->AsIdentifier() && !node->AsAccessor()) { skip = true; - continue; + break; } } - if (skip) + if (skip) { continue; + } + // Save the original line number so that we can re-assign ranges. We assume // they're contiguous lines because GetSortRanges() does so above. We need // to re-assign these line numbers primiarily because `gn format` uses them