anton-afanasyev marked 3 inline comments as done.
anton-afanasyev added inline comments.
================
Comment at: clang/lib/Parse/ParseAST.cpp:154
if (HaveLexer) {
+ llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
P.Initialize();
----------------
takuto.ikuta wrote:
> Remove StringRef?
I use `StringRef("")` to explicitly cast to one of overloaded
`TimeScope(StringRef, StringRef)` and `TimeScope(StringRef,
fuction_ref(std::string()))`.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:103
+
+ *OS << "{ \"traceEvents\": [\n";
+
----------------
takuto.ikuta wrote:
> Don't we want to use json utility for this?
> https://github.com/llvm/llvm-project/blob/master/llvm/include/llvm/Support/JSON.h
>
This implementation looks compact and fast. I've read proposal for this library
https://docs.google.com/document/d/1OEF9IauWwNuSigZzvvbjc1cVS1uGHRyGTXaoy3DjqM4
, it's recent, so I'm not sure it's stable and speed optimized. Do you actually
can advise it? I can do it in the next refactor commit as well.
================
Comment at: llvm/lib/Support/TimeProfiler.cpp:44
+ default:
+ if (isPrint(C)) {
+ OS += C;
----------------
takuto.ikuta wrote:
> include StringExtras.h for this?
Should one do it? It's already implicitly included.
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
