[PATCH] D58675: [clang] Adds `-ftime-trace` option to clang that produces Chrome `chrome://tracing` compatible JSON profiling output dumps

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm

I think this is ready. We can adjust the overloads after the fact. I'd like to get the feature in so we can make improvements independently.



================
Comment at: clang/lib/Parse/ParseAST.cpp:154
   if (HaveLexer) {
+    llvm::TimeTraceScope TimeScope("Frontend", StringRef(""));
     P.Initialize();
----------------
anton-afanasyev wrote:
> takuto.ikuta wrote:
> > anton-afanasyev wrote:
> > > 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()))`.
> > I think function_ref(std::string()) does not have constructor receiving StringRef, so I feel explicit cast is redundant.
> > 
> The compiler tries to use function_ref<..>(Callable&&, ...) constructor and instantiates `function_ref<std::string()>::function_ref<char const (&)[1]>`, so one gets error like:
> ```
> error: call to constructor of 'llvm::TimeTraceScope' is ambiguous
>     llvm::TimeTraceScope TimeScope("Frontend", "");
>                          ^         ~~~~~~~~~~~~~~
> .../include/llvm/Support/TimeProfiler.h:54:3: note: candidate constructor
>   TimeTraceScope(StringRef Name, StringRef Detail) {
>   ^
> .../include/llvm/Support/TimeProfiler.h:58:3: note: candidate constructor
>   TimeTraceScope(StringRef Name, llvm::function_ref<std::string()> Detail) {
>   ^
> 
> ```
Oh no. We are too clever for ourselves. :(

So, we'd need to make the enable_if logic in the function_ref constructor more clever to prevent it from being instantiated for non-callables.


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

https://reviews.llvm.org/D58675