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