From 520f0a549f8ac2f2bfafabd77b7269978961f91c Mon Sep 17 00:00:00 2001 From: Oleh Prypin Date: Mon, 22 Jul 2024 07:43:43 -0700 Subject: [PATCH] Remove some unnecessary uses of utilities PiperOrigin-RevId: 654745047 --- pytype/typegraph/map_util.h | 11 --------- pytype/typegraph/map_util_test.cc | 7 ------ pytype/typegraph/memory_util.h | 9 -------- pytype/typegraph/solver.cc | 38 +++++++++++++------------------ pytype/typegraph/typegraph.cc | 10 ++++---- 5 files changed, 20 insertions(+), 55 deletions(-) diff --git a/pytype/typegraph/map_util.h b/pytype/typegraph/map_util.h index 7bf16f364..b59cb9c81 100644 --- a/pytype/typegraph/map_util.h +++ b/pytype/typegraph/map_util.h @@ -63,17 +63,6 @@ V FindPtrOrNull(const M& map, const K& key) { return nullptr; } -// FindOrDefault returns a const reference to the value associated with the -// given key if it exists, otherwise returns a const reference to the given -// default value. -template -const V& FindOrDefault(const M& map, const K& key, const V& value) { - auto it = map.find(key); - if (it != map.end()) - return it->second; - return value; -} - } // namespace map_util } // namespace devtools_python_typegraph diff --git a/pytype/typegraph/map_util_test.cc b/pytype/typegraph/map_util_test.cc index ce9c20e4f..c82637aaf 100644 --- a/pytype/typegraph/map_util_test.cc +++ b/pytype/typegraph/map_util_test.cc @@ -35,13 +35,6 @@ TEST(MapUtilTest, FindPtrOrNullTest) { EXPECT_EQ(*res, val); } -TEST(MapUtilTest, FindOrDefaultTest) { - std::unordered_map m; - EXPECT_EQ(FindOrDefault(m, 1, 2), 2); - m[1] = 3; - EXPECT_EQ(FindOrDefault(m, 1, 2), 3); -} - } // namespace } // namespace map_util diff --git a/pytype/typegraph/memory_util.h b/pytype/typegraph/memory_util.h index 3c8044f07..13e3e37c1 100644 --- a/pytype/typegraph/memory_util.h +++ b/pytype/typegraph/memory_util.h @@ -8,7 +8,6 @@ #define PYTYPE_TYPEGRAPH_MEMORY_UTIL_H_ #include -#include namespace devtools_python_typegraph { @@ -21,14 +20,6 @@ std::unique_ptr make_unique(Args&&... args) { return std::unique_ptr(new T(std::forward(args)...)); } -// WrapUnique wraps a pointer in a unique_ptr. Unlike make_unique, it can be -// used for classes that have private constructors. -template -std::unique_ptr WrapUnique(T* ptr) { - static_assert(!std::is_array::value, "array types are unsupported"); - static_assert(std::is_object::value, "non-object types are unsupported"); - return std::unique_ptr(ptr); -} } // namespace memory_util } // namespace devtools_python_typegraph diff --git a/pytype/typegraph/solver.cc b/pytype/typegraph/solver.cc index ef06b88d7..58cfa10fd 100644 --- a/pytype/typegraph/solver.cc +++ b/pytype/typegraph/solver.cc @@ -53,8 +53,7 @@ static std::vector remove_finished_goals(const CFGNode* pos, std::inserter(new_goals, new_goals.begin()), pointer_less()); std::vector> stack; - stack.push_back( - std::make_tuple(goals_to_remove, seen_goals, removed_goals, new_goals)); + stack.emplace_back(goals_to_remove, seen_goals, removed_goals, new_goals); std::vector results; while (!stack.empty()) { std::tie( @@ -68,24 +67,22 @@ static std::vector remove_finished_goals(const CFGNode* pos, goals_to_remove.erase(goals_to_remove.begin()); if (seen_goals.count(goal)) { // Only process a goal once, to prevent infinite loops. - stack.push_back(std::make_tuple( - goals_to_remove, seen_goals, removed_goals, new_goals)); + stack.emplace_back(goals_to_remove, seen_goals, removed_goals, new_goals); continue; } seen_goals.insert(goal); const auto* origin = goal->FindOrigin(pos); if (!origin) { new_goals.insert(goal); - stack.push_back(std::make_tuple( - goals_to_remove, seen_goals, removed_goals, new_goals)); + stack.emplace_back(goals_to_remove, seen_goals, removed_goals, new_goals); continue; } removed_goals.insert(goal); for (const auto& source_set : origin->source_sets) { GoalSet next_goals_to_remove(goals_to_remove); next_goals_to_remove.insert(source_set.begin(), source_set.end()); - stack.push_back(std::make_tuple( - next_goals_to_remove, seen_goals, removed_goals, new_goals)); + stack.emplace_back(std::move(next_goals_to_remove), seen_goals, + removed_goals, new_goals); } } return results; @@ -171,7 +168,6 @@ const CFGNode* PathFinder::FindHighestReachableWeight( std::vector stack; stack.insert(stack.end(), start->incoming().begin(), start->incoming().end()); int best_weight = -1; - int weight; const CFGNode* best_node = nullptr; const CFGNode* node; while (!stack.empty()) { @@ -180,9 +176,9 @@ const CFGNode* PathFinder::FindHighestReachableWeight( if (node == start) // Don't allow loops back to the start. continue; - weight = map_util::FindOrDefault(weight_map, node, -1); - if (weight > best_weight) { - best_weight = weight; + const auto weight = weight_map.find(node); + if (weight != weight_map.end() && weight->second > best_weight) { + best_weight = weight->second; best_node = node; } if (!seen.insert(node).second) continue; @@ -216,9 +212,9 @@ QueryResult PathFinder::FindNodeBackwards( blocked_.insert(shortest_path.begin(), shortest_path.end()); std::unordered_map weights; int w = 0; - std::deque::const_iterator it = shortest_path.cbegin(); - for (; it != shortest_path.cend(); w++, it++) - weights[*it] = w; + for (const auto& x : shortest_path) { + weights[x] = w++; + } std::deque path; const CFGNode* node = start; while (true) { @@ -249,15 +245,13 @@ SolverMetrics Solver::CalculateMetrics() const { bool Solver::GoalsConflict(const internal::GoalSet& goals) const { std::unordered_map variables; for (const Binding* goal : goals) { - const Binding* existing = map_util::FindPtrOrNull(variables, - goal->variable()); - if (existing) { - CHECK(existing != goal) << "Internal error. Duplicate goal."; - CHECK(existing->data() != goal->data()) << - "Internal error. Duplicate data across bindings."; + const auto& [it, inserted] = variables.emplace(goal->variable(), goal); + if (!inserted) { + CHECK(it->second != goal) << "Internal error. Duplicate goal."; + CHECK(it->second->data() != goal->data()) + << "Internal error. Duplicate data across bindings."; return true; } - variables[goal->variable()] = goal; } return false; } diff --git a/pytype/typegraph/typegraph.cc b/pytype/typegraph/typegraph.cc index 255939a12..7365d06a7 100644 --- a/pytype/typegraph/typegraph.cc +++ b/pytype/typegraph/typegraph.cc @@ -28,7 +28,7 @@ CFGNode* Program::NewCFGNode(std::string name, Binding* condition) { int n = backward_reachability_->add_node(); CHECK(n == node_nr) << "internal error: wrong reachability cache node count."; - auto node = memory_util::WrapUnique(new CFGNode( + std::unique_ptr node(new CFGNode( this, std::move(name), node_nr, condition, backward_reachability_.get())); CFGNode* np = node.get(); cfg_nodes_.push_back(std::move(node)); @@ -37,7 +37,7 @@ CFGNode* Program::NewCFGNode(std::string name, Binding* condition) { Variable* Program::NewVariable() { LOG(DEBUG) << "Creating Variable v" << next_variable_id_; - auto u = memory_util::WrapUnique(new Variable(this, next_variable_id_)); + std::unique_ptr u(new Variable(this, next_variable_id_)); next_variable_id_ += 1; Variable* up = u.get(); variables_.push_back(std::move(u)); @@ -284,10 +284,8 @@ Binding* Variable::FindOrAddBinding(const BindingData& data) { } void Variable::RegisterBindingAtNode(Binding* binding, const CFGNode* node) { - if (!map_util::ContainsKey(cfg_node_to_bindings_, node)) { - cfg_node_to_bindings_[node] = SourceSet(); - } - cfg_node_to_bindings_[node].insert(binding); + const auto& [it, _] = cfg_node_to_bindings_.emplace(node, SourceSet()); + it->second.insert(binding); } Binding* Variable::AddBinding(const BindingData& data) {