[PATCH] D80029: Fix quadratic LexicalScopes::constructScopeNest(...), NFC

[Jeremy Morse via Phabricator via llvm-commits]

jmorse accepted this revision.
jmorse added a comment.

LGTM with one nit inline and Davids recommendations done. Thanks for finding this! I can land it when you're ready, please provide a name and email to be set as the git commit author.



================
Comment at: llvm/lib/CodeGen/LexicalScopes.cpp:245
+      auto &ChildScope = Children[i];
+      // Because of tree structure, !ChildScope->getDFSIn().
+      WorkStack.push_back(std::make_pair(ChildScope, 0));
----------------
IMO this comment isn't necessary, because it's describing the relationship between the new code and old code, which is best put in a commit message. After this lands, the DFSIn / DFSOut values of an unvisited scope shouldn't be something a reader would consider at all.

That being said, would you be able to add a comment about the overall purpose of the function somewhere, i.e. "Visit each scope in depth-first order, with the current exploration state kept as a tree in WorkStack", or similar.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80029