[llvm] 1c15377 - Recommit: CFGDiff: Simplify/common the begin/end implementations to use a common range helper""
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 19 18:38:40 PDT 2020
Author: David Blaikie
Date: 2020-03-19T18:21:14-07:00
New Revision: 1c1537749615a9be98a33482cdf6bf6f602f1eed
URL: https://github.com/llvm/llvm-project/commit/1c1537749615a9be98a33482cdf6bf6f602f1eed
DIFF: https://github.com/llvm/llvm-project/commit/1c1537749615a9be98a33482cdf6bf6f602f1eed.diff
LOG: Recommit: CFGDiff: Simplify/common the begin/end implementations to use a common range helper""
(would be nice to revisit the CFG traits and change them to use ranges
rather than begin/end - if anyone wants to do that refactor)
Also use more auto because writing the names of range utilty iterators
isn't helping readability here - they're sort of implementation details
for the most part, especially once you nest a few different filtering
and adapting iterators.
The fix (shooting from the hip since I couldn't reproduce this locally)
was to capture by value in a lambda used in a filtering iterator -
because the iterator would persist beyond the lifetime of the function
(as the iterators are returned to callers).
Originally committed in 79a7ed92a9b135212a6a271dd8dbc625038c8f06.
This was reverted in 4a7f2032a350bc7eefd26709563f65216df3e2ce.
Added:
Modified:
llvm/include/llvm/IR/CFGDiff.h
Removed:
################################################################################
diff --git a/llvm/include/llvm/IR/CFGDiff.h b/llvm/include/llvm/IR/CFGDiff.h
index f40434cc5d4e..cc38addd058e 100644
--- a/llvm/include/llvm/IR/CFGDiff.h
+++ b/llvm/include/llvm/IR/CFGDiff.h
@@ -158,58 +158,44 @@ template <typename GraphT, bool InverseGraph = false, bool InverseEdge = false,
typename GT = GraphTraits<GraphT>>
struct CFGViewChildren {
using DataRef = const GraphDiff<typename GT::NodeRef, InverseGraph> *;
- using RawNodeRef = typename GT::NodeRef;
- using NodeRef = std::pair<DataRef, RawNodeRef>;
-
- using ExistingChildIterator =
- WrappedPairNodeDataIterator<typename GT::ChildIteratorType, NodeRef,
- DataRef>;
- struct DeletedEdgesFilter {
- RawNodeRef BB;
- DeletedEdgesFilter(RawNodeRef BB) : BB(BB){};
- bool operator()(NodeRef N) const {
- return !N.first->ignoreChild(BB, N.second, InverseEdge);
- }
- };
- using FilterExistingChildrenIterator =
- filter_iterator<ExistingChildIterator, DeletedEdgesFilter>;
-
- using vec_iterator = typename SmallVectorImpl<RawNodeRef>::const_iterator;
- using AddNewChildrenIterator =
- WrappedPairNodeDataIterator<vec_iterator, NodeRef, DataRef>;
- using ChildIteratorType =
- concat_iterator<NodeRef, FilterExistingChildrenIterator,
- AddNewChildrenIterator>;
-
- static ChildIteratorType child_begin(NodeRef N) {
- auto InsertVec = N.first->getAddedChildren(N.second, InverseEdge);
- // filter iterator init:
- auto firstit = make_filter_range(
- make_range<ExistingChildIterator>({GT::child_begin(N.second), N.first},
- {GT::child_end(N.second), N.first}),
- DeletedEdgesFilter(N.second));
- // new inserts iterator init:
- auto secondit = make_range<AddNewChildrenIterator>(
- {InsertVec.begin(), N.first}, {InsertVec.end(), N.first});
+ using NodeRef = std::pair<DataRef, typename GT::NodeRef>;
- return concat_iterator<NodeRef, FilterExistingChildrenIterator,
- AddNewChildrenIterator>(firstit, secondit);
+ template<typename Range>
+ static auto makeChildRange(Range &&R, DataRef DR) {
+ using Iter = WrappedPairNodeDataIterator<decltype(std::forward<Range>(R).begin()), NodeRef, DataRef>;
+ return make_range(Iter(R.begin(), DR), Iter(R.end(), DR));
}
- static ChildIteratorType child_end(NodeRef N) {
- auto InsertVec = N.first->getAddedChildren(N.second, InverseEdge);
+ static auto children(NodeRef N) {
+
// filter iterator init:
- auto firstit = make_filter_range(
- make_range<ExistingChildIterator>({GT::child_end(N.second), N.first},
- {GT::child_end(N.second), N.first}),
- DeletedEdgesFilter(N.second));
+ auto R = make_range(GT::child_begin(N.second), GT::child_end(N.second));
+ // This lambda is copied into the iterators and persists to callers, ensure
+ // captures are by value or otherwise have sufficient lifetime.
+ auto First = make_filter_range(makeChildRange(R, N.first), [N](NodeRef C) {
+ return !C.first->ignoreChild(N.second, C.second, InverseEdge);
+ });
+
// new inserts iterator init:
- auto secondit = make_range<AddNewChildrenIterator>(
- {InsertVec.end(), N.first}, {InsertVec.end(), N.first});
+ auto InsertVec = N.first->getAddedChildren(N.second, InverseEdge);
+ auto Second = makeChildRange(InsertVec, N.first);
+
+ auto CR = concat<NodeRef>(First, Second);
+ // concat_range contains references to other ranges, returning it would
+ // leave those references dangling - the iterators contain
+ // other iterators by value so they're safe to return.
+ return make_range(CR.begin(), CR.end());
+ }
- return concat_iterator<NodeRef, FilterExistingChildrenIterator,
- AddNewChildrenIterator>(firstit, secondit);
+ static auto child_begin(NodeRef N) {
+ return children(N).begin();
}
+
+ static auto child_end(NodeRef N) {
+ return children(N).end();
+ }
+
+ using ChildIteratorType = decltype(child_end(std::declval<NodeRef>()));
};
template <typename T, bool B>
More information about the llvm-commits
mailing list