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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
