rnk added a comment.
Other than the lifetime issue, I think this is basically ready.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2014
+ llvm::TimeTraceScope TimeScope("InstantiateClass", [&]() {
+ return Instantiation->getQualifiedNameAsString().c_str();
+ });
----------------
Shoot, I think the callable should probably return std::string instead of
StringRef, otherwise this looks like it will be a use-after-free. You allocate
a temporary std::string, get a pointer to the characters, return, the string is
destroyed, and then UAF.
With a std::string return type, you won't need `.c_str()`.
================
Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4089
+ llvm::TimeTraceScope TimeScope("InstantiateFunction", [&]() {
+ return Function->getQualifiedNameAsString().c_str();
+ });
----------------
`.c_str()` is unnecessary
================
Comment at: llvm/include/llvm/Support/TimeProfiler.h:54-58
+ TimeTraceScope(const char *Name, const char *Detail) {
+ if (TimeTraceProfilerInstance != nullptr)
+ timeTraceProfilerBegin(Name, Detail);
+ }
+ TimeTraceScope(const char *Name, llvm::function_ref<StringRef()> Detail) {
----------------
I think you can replace `const char *` in these prototypes with StringRef to
save a few `.data()` calls at some call sites.
As mentioned before, I think the callable needs to return `std::string`, though.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58675/new/
https://reviews.llvm.org/D58675
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
