Enable string-views for use with Tracelogging #589
Conversation
|
The exact output from the asan tests (x64) are: Which maps to: ... so this is llvm/llvm-project#125105 |
jonwis
changed the title
| #include <string> | ||
| #include <vector> | ||
| #include <utility> | ||
| #include <TraceLoggingProvider.h> |
There was a problem hiding this comment.
Technically this is illegally reaching into internal implementation details of TraceLoggingProvider.h.
Probably the right thing to do is:
- Modify TraceLoggingProvider.h to expose enough "supported" machinery to support some C++ constructs.
- Modify WIL to use the supported mechanism if present, or to poly-fill the machinery if not present. (The poly-fill will only be used on old versions of TraceLoggingProvider.h, so future changes to TraceLoggingProvider.h internals will not break the poly-fill.
| #if __cpp_lib_string_view >= 201606L | ||
|
|
||
| template <typename TContainer> | ||
| struct _tlgWrapBufferStlContainer |
There was a problem hiding this comment.
The _tlg prefix is used to signal "TraceLoggingProvider.h-internal machinery". This prefix should not be used for "WIL-internal machinery", even if it is "WIL-internal TraceLogging-related" machinery.
| }; | ||
|
|
||
| template <typename TChar> | ||
| struct _tlgTypeMapStlString |
| }; | ||
|
|
||
| template <typename TChar, typename TTraits> | ||
| TLG_INLINE auto _tlg_CALL _tlgWrapAuto(std::basic_string_view<TChar, TTraits> const& value) |
There was a problem hiding this comment.
Don't use _tlg_CALL here -- it's an unnecessary use of TraceLoggingProvider.h internal machinery. Instead, use WINAPI or STDCALL or similar.
| } | ||
|
|
||
| #define TraceLoggingStringView(pValue, ...) _tlgArgAuto(static_cast<std::string_view>(pValue), __VA_ARGS__) | ||
| #define TraceLoggingWideStringView(pValue, ...) _tlgArgAuto(static_cast<std::wstring_view>(pValue), __VA_ARGS__) |
There was a problem hiding this comment.
The parameter to this macro should not be a pointer, so probably shouldn't be named pValue.
| typedef UINT8 _tlgTypeType0; | ||
| typedef UINT16 _tlgTypeType1; | ||
| static bool const _tlgIsSimple = false; | ||
| static TlgIn_t const _tlgViewIn = wistd::is_same_v<TChar, char> ? TlgInCOUNTEDANSISTRING : TlgInCOUNTEDSTRING; |
There was a problem hiding this comment.
Should probably be based on sizeof(TChar) so it can do something reasonable with char8_t and char16_t. (And it should error-out for sizes other than 1 and 2.)
| }; | ||
|
|
||
| template <typename TChar> | ||
| struct _tlgTypeMapStlString |
There was a problem hiding this comment.
Should probably have a specialization for char8_t that sets OutType to UTF8.
| }; | ||
|
|
||
| template <typename TChar, typename TTraits> | ||
| TLG_INLINE auto _tlg_CALL _tlgWrapAuto(std::basic_string_view<TChar, TTraits> const& value) |
There was a problem hiding this comment.
Don't need TLG_INLINE here -- it's an unnecessary obfuscation. Just use inline.
2 participants
We have a lot of tracelogging, and we have a lot of things that are string views to log. Unfortunately, without direct support for counted strings in TLG, we see people doing this:
Or worse,
winrt::hstring(x).c_str()- all to get the null-terminated form that TraceLoggingWrite/TraceLoggingXString require.This change adds specializations for the
_tlgAutotemplates, sowil::zwstring_view,std::wstring_view,std::string_view,std::wstring,std::string, andwinrt::hstringcan all be used withTraceLoggingValue(). They can also be used as the parameter-type for theDEFINE_EVENT_*family of macro-stamped-out events.A better test would use one of the ETW watchers to verify that the event got logged, but a manual use of
xperf -start MyLogger -on bd2bf191-ac1a-4732-b563-bb3e3006f617then run the tests, thenxperf -stop -d something.etland opensomething.etlwith WPA.The infrastructure is also present to support passing arrays/spans of scalar types, but left unimplemented for now.