[PATCH] D112696: CycleInfo: Introduce cycles as a generalization of loops

[Nicolai Hähnle via Phabricator via llvm-commits]

nhaehnle added a comment.

LGTM



================
Comment at: llvm/include/llvm/ADT/GenericCycleImpl.h:34
+template <typename ContextT>
+bool GenericCycle<ContextT>::contains(const GenericCycle *B) const {
+  if (!B)
----------------
sameerds wrote:
> nhaehnle wrote:
> > Other code in this file uses the new style function definition syntax, so perhaps do so here as well to be consistent?
> I am not sure what you mean here ... this definition passes clang-format and clang-tidy. If you mean the lowercase `::contains`, then that is valid camelBack where the first letter must be lowercase.
I meant the `auto foo(...) -> T` syntax.


================
Comment at: llvm/include/llvm/ADT/GenericCycleInfo.h:10
+/// \file
+/// \brief Find all cycles in a control-flow graph, including irreducible loops.
+//
----------------
Maybe add a "See docs/CycleTerminology.rst for a high-level overview." comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112696