[PATCH] D47196: [Time-report ](2): Recursive timers in Clang

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Adding startFrontendTimer/stopFrontendTimer helps a little, but it's still difficult to match a given startFrontendTimer to the corresponding stopFrontendTimer because they're in completely different functions in some cases.  Do they really need to be scattered like that?  If they do, please add comments so someone reading the code can match them up.

Does getFrontendFunctionTimeCtx need to be a template, given it's always instantiated with `const FunctionDecl *`?



================
Comment at: include/clang/Frontend/Utils.h:338
+      }
+      if (isFirstValid(Result.first) && Result.second > 0.00001) {
+        FrontendTimes.push_back(Result);
----------------
Why 0.00001? Should it be configurable?


================
Comment at: include/clang/Frontend/Utils.h:385
+          range.first++;
+        }
+        FinalTimes.push_back({E, dist});
----------------
This is merging together the times for functions with the same name? Why is that necessary?

This function could use a few comments describing what it's doing.


================
Comment at: include/clang/Frontend/Utils.h:462
+    if (Ctx && Ctx->IsValid) {
+      //      Ctx->stopFrontendTimer(true, {T(), -1.0});
+      Ctx->stopFrontendTimer();
----------------
Commented-out code?


https://reviews.llvm.org/D47196