[PATCH] D74801: [ADT][NFC] SCCIterator: Change hasLoop() to hasCycle()

Stefanos Baziotis via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 18 15:38:37 PST 2020


baziotis marked an inline comment as done.
baziotis added a comment.

In D74801#1881693 <https://reviews.llvm.org/D74801#1881693>, @lebedev.ri wrote:

> While i certainly agree that it's kinda confusing, i'm not really sure this is conceptually correct change.
>  https://en.wikipedia.org/wiki/Loop_(graph_theory)
>
> > In graph theory, a loop (also called a self-loop or a "buckle") is an edge that connects a vertex to itself. A simple graph contains no loops.
>
> which is exactly the case here.
>
> https://en.wikipedia.org/wiki/Cycle_(graph_theory)
>  seems more generic than that.


Yes, I agree, but it was the closest word I could find that is not "loop" but still gets the point across. I mean technically maybe the better term would be "hasALoopButNotNecessarilyANormalLoop()" but well... :P
In any case, that was an experience of mine as a newcomer. If you think it's not worth it, so be it. :)



================
Comment at: llvm/tools/opt/PrintSCC.cpp:82-83
       errs() << (*I)->getName() << ", ";
-    if (nextSCC.size() == 1 && SCCI.hasLoop())
-      errs() << " (Has self-loop).";
+    if (nextSCC.size() == 1 && SCCI.hasCycle())
+      errs() << " (Has self-cycle).";
   }
----------------
lebedev.ri wrote:
> err, i was specifically talking about this
Maybe we can keep the "Has self-loop here".


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D74801/new/

https://reviews.llvm.org/D74801





More information about the llvm-commits mailing list