[PATCH] D136549: [clang] Fix time profile in "isIntegerConstantExpr"

[Joe Loser via Phabricator via cfe-commits]

jloser accepted this revision.
jloser added a comment.
This revision is now accepted and ready to land.

LGTM.  Thanks for the fix.

I did leave an open question or two though, but it is non-blocking.



================
Comment at: clang/lib/AST/ExprConstant.cpp:15908
 
-  llvm::TimeTraceScope TimeScope("isIntegerConstantExpr", [&] {
-    return Loc->printToString(Ctx.getSourceManager());
-  });
+  ExprTimeTraceScope TimeScope(this, Ctx, "isIntegerConstantExpr");
 
----------------
**Question** This looks like the right fix for this call site.  Are the other two uses of `llvm::TimeTraceScope` as a local variable subject to similar problems? I don't think so since we don't try to get the location explicitly, but want to confirm.


================
Comment at: clang/unittests/Support/TimeProfilerTest.cpp:209
+  setupProfiler();
+  ASSERT_TRUE(compileFromString(Code, "-std=c99", "test.c"));
+  std::string Json = teardownProfiler();
----------------
**Question** Is adding the ability to plumb the standards mode just useful for this bug fix in the sense of reducing the trace graph output of the AST?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136549