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
