[PATCH] D77341: [DomTree] Replace ChildrenGetter with GraphTraits over GraphDiff.
Alina Sbirlea via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 7 16:55:04 PDT 2020
asbirlea marked 4 inline comments as done.
asbirlea added inline comments.
================
Comment at: llvm/include/llvm/Support/GenericDomTreeConstruction.h:325
+ auto &GD = BUI ? BUI->PreViewCFG : EmptyGD;
+ return !empty(children<GraphDiffNodePair>({&GD, N}));
}
----------------
kuhar wrote:
> This pattern repeats a few times. Could it be pushed into a helper function to get something like this?
>
> ```
> return !empty(children<GraphDiffNodePair>(GetGraphDiffNodePair(BUI, N)));
> ```
>
My dilemma here is how to not allocate a GraphDiffT instance. There are 4 cases where the pattern is inside a static method and once in a class method. I used a stack var for the 4 cases and a unique_ptr for the class method.
Sure, I can do:
```
static auto getGDNPair(BUI, EmptyGD, N) {
return std::make_pair<>(BUI ? BUI->PreViewCFG : EmptyGD, N);
}
{
...
GraphDiffT EmptyGD;
return !empty(children<GraphDiffNodePair>(getGDNPair(BUI, &EmptyGD, N)));
}
```
But it felt like moving one line at the callsite to a oneline helper is not making things much better.
I was hoping for something more along the lines of:
```
return !empty(children<GraphDiffNodePair>({getGD(BUI), N});
```
Or, ensure BUI is always non-null, and simplify to:
```
return !empty(children<GraphDiffNodePair>({BUI->PreViewCFG, N});
```
Thoughts?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77341/new/
https://reviews.llvm.org/D77341
More information about the cfe-commits
mailing list