[PATCH] D23522: [ADT] Change PostOrderIterator to use NodeRef. NFC.

Tim Shen via llvm-commits llvm-commits at lists.llvm.org
Mon Aug 15 14:49:39 PDT 2016


timshen added inline comments.

================
Comment at: include/llvm/ADT/PostOrderIterator.h:122
@@ -123,3 +121,3 @@
       : po_iterator_storage<SetType, ExtStorage>(S) {
-    if (this->insertEdge((NodeType*)nullptr, BB)) {
+    if (this->insertEdge(Optional<NodeRef>(), BB)) {
       VisitStack.push_back(std::make_pair(BB, GT::child_begin(BB)));
----------------
dblaikie wrote:
> timshen wrote:
> > Yeah, the main reason I don't want that is it aggressively generalize the insertEdge's interface.
> Not sure I follow - it doesn't really make the interface more permissive (if there were overloads then you'd need some SFINAE to promote the internal template errors out onto the interface to ensure overload resolution did the right thing is the only big wrinkle) - so far as I can see.
> 
> Anything that can be implicitly converted to Optional<NodeRef> seems to be what the function is trying to accept.
Assuming that the user relies on template argument deduction, then the user has to pass in an Optional<...> object, right? This limits the interface to take Optional objects, not any T that is convertible to an Optional object.

If the user doesn't rely on template argument deduction, they are the same.


https://reviews.llvm.org/D23522





More information about the llvm-commits mailing list