Enable string-views for use with Tracelogging #589

jonwis · 2025-12-30T07:23:52Z

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:

void do_thing(std::wstring_view x) {
    if (condition) {
        TraceLoggingWrite(g_Provider, TraceLoggingValue(std::wstring(x).c_str()));
    }
}

Or worse, winrt::hstring(x).c_str() - all to get the null-terminated form that TraceLoggingWrite/TraceLoggingXString require.

This change adds specializations for the _tlgAuto templates, so wil::zwstring_view, std::wstring_view, std::string_view, std::wstring, std::string, and winrt::hstring can all be used with TraceLoggingValue(). They can also be used as the parameter-type for the DEFINE_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-bb3e3006f617 then run the tests, then xperf -stop -d something.etl and open something.etl with WPA.

The infrastructure is also present to support passing arrays/spans of scalar types, but left unimplemented for now.

Note: This change updates CMakePresets.json - in a stock install of Kitware.Cmake and LLVM.LLVM, the current presets fail; putting cacheVariables into a buildPreset is not support - it has to go into a configurePreset instead.

Note: This change updates the test script to allow selective disablement of the ASAN tests. My local machine is already on Win11, and hits the issues referenced in llvm/llvm-project#111638 and related. Sounds like my NTDLL has the new compiler codegen enabled.

…gging-for-strings

jonwis · 2025-12-30T07:38:04Z

The exact output from the asan tests (x64) are:

Running sanitize-address tests...
==17176==interception_win: unhandled instruction at 0x7ffa0d133000: 44 0f b6 1a 4c 8b d2 48
==17176==interception_win: unhandled instruction at 0x7ffa0d133000: 44 0f b6 1a 4c 8b d2 48

Which maps to:

0:000> ln 0x7ffa0d133000
Browse module
Set bu breakpoint

(00007ffa`0d133000)   ucrtbase!strstr   |  (00007ffa`0d133120)   ucrtbase!initialize_c

... so this is llvm/llvm-project#125105

idigdoug · 2025-12-31T00:05:02Z

include/wil/stl.h

 #include <string>
 #include <vector>
 #include <utility>
+#include <TraceLoggingProvider.h>


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.

idigdoug · 2025-12-31T00:06:41Z

include/wil/stl.h

+#if __cpp_lib_string_view >= 201606L
+
+template <typename TContainer>
+struct _tlgWrapBufferStlContainer


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.

idigdoug · 2025-12-31T00:12:13Z

include/wil/stl.h

+};
+
+template <typename TChar>
+struct _tlgTypeMapStlString


Should not use _tlg prefix for this.

idigdoug · 2025-12-31T00:23:41Z

include/wil/stl.h

+};
+
+template <typename TChar, typename TTraits>
+TLG_INLINE auto _tlg_CALL _tlgWrapAuto(std::basic_string_view<TChar, TTraits> const& value)


Don't use _tlg_CALL here -- it's an unnecessary use of TraceLoggingProvider.h internal machinery. Instead, use WINAPI or STDCALL or similar.

why would we need a calling convention for this template function? is it passed as a function pointer somewhere?

In practice, it's probably not particularly significant. However, it is possible for two LIBs to use different default calling conventions and both include the header. In that case, the linker will see two distinct inline functions, and that can be confusing or sub-optimal. I just find it to be best practice to put a calling convention on any non-member function declaration in a header.

ok. I don't think we put calling conventions on standalone functions anywhere else in wil :)

idigdoug · 2025-12-31T00:27:35Z

include/wil/stl.h

+}
+
+#define TraceLoggingStringView(pValue, ...) _tlgArgAuto(static_cast<std::string_view>(pValue), __VA_ARGS__)
+#define TraceLoggingWideStringView(pValue, ...) _tlgArgAuto(static_cast<std::wstring_view>(pValue), __VA_ARGS__)


The parameter to this macro should not be a pointer, so probably shouldn't be named pValue.

idigdoug · 2025-12-31T00:30:24Z

include/wil/stl.h

+    typedef UINT8 _tlgTypeType0;
+    typedef UINT16 _tlgTypeType1;
+    static bool const _tlgIsSimple = false;
+    static TlgIn_t const _tlgViewIn = wistd::is_same_v<TChar, char> ? TlgInCOUNTEDANSISTRING : TlgInCOUNTEDSTRING;


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.)

idigdoug · 2025-12-31T00:31:33Z

include/wil/stl.h

+};
+
+template <typename TChar>
+struct _tlgTypeMapStlString


Should probably have a specialization for char8_t that sets OutType to UTF8.

idigdoug · 2025-12-31T00:32:51Z

include/wil/stl.h

+};
+
+template <typename TChar, typename TTraits>
+TLG_INLINE auto _tlg_CALL _tlgWrapAuto(std::basic_string_view<TChar, TTraits> const& value)


Don't need TLG_INLINE here -- it's an unnecessary obfuscation. Just use inline.

dunhor · 2026-01-04T07:28:37Z

scripts/runtests.cmd

 :: so we _can_ build the ASan tests, which is at least something. For now we're going to handle this limitation here and
 :: avoid running the ASan test in this specific scenario
-set RUN_ASAN_TEST=1
+if not %RUN_ASAN_TEST%=='' set RUN_ASAN_TEST=1


I assume your intent is to initialize if not set, in which case the not seems out of place. Additionally, you probably want to surround with quotes so that once variable substitution takes place, you don't have something empty on the left hand side, which normally causes errors and is probably why CI is failing. E.g.

Suggested change

if not %RUN_ASAN_TEST%=='' set RUN_ASAN_TEST=1

if "%RUN_ASAN_TEST%"=="" set RUN_ASAN_TEST=1

keith-horton · 2026-01-05T18:03:58Z

include/wil/stl.h

+
+    TContainer const& view;
+
+    __forceinline explicit _tlgWrapBufferStlContainer(_In_ TContainer const& ref) : view(ref)


does this need our special __forceinline keyword? can we just use the standard 'inline' keyword?

keith-horton · 2026-01-05T18:04:40Z

include/wil/stl.h

+
+    TContainer const& view;
+
+    __forceinline explicit _tlgWrapBufferStlContainer(_In_ TContainer const& ref) : view(ref)


as this is a template parameter, can we use a 'Universal Reference' with std::forward?

I don't think that achieves anything here. Universal reference is so that you can accept both const and non-const references, or so you can accept both non-const-rvalue and non-const-lvalue references. We only need to be able to accept a const reference, so no fancy universal reference is needed.

that's because moves don't buy us anything for these templated types vs. making copies?

There are no copies. It's a const ref all the way down.

oops. yep. sorry, missed that!

Jon - feel free to close this. I can't seem to resolve/close it myself :(

keith-horton · 2026-01-05T18:05:57Z

include/wil/stl.h

+{
+    typedef UINT8 _tlgTypeType0;
+    typedef UINT16 _tlgTypeType1;
+    static bool const _tlgIsSimple = false;


can these be constexpr?

There isn't really any significant difference between class scope static bool const and static bool constexpr. A constexpr would allow more complex variables to be used here (const static only allows trivial scalars to be used without a corresponding extern), but we only need a bool.

sure. I tend to make everything constexpr that can be supported, as I don't assume what the compiler can optimize / change where it puts such things - so I make it the rule, not the exception :)

it's clearly not a bit deal to go either way to me.

keith-horton · 2026-01-05T18:06:44Z

include/wil/stl.h

+};
+
+template <typename TChar, typename TTraits>
+TLG_INLINE auto _tlg_CALL _tlgWrapAuto(std::basic_string_view<TChar, TTraits> const& value)


why would we need a calling convention for this template function? is it passed as a function pointer somewhere?

keith-horton · 2026-01-05T18:08:07Z

include/wil/stl.h

+};
+
+template <typename TChar, typename TTraits>
+TLG_INLINE auto _tlg_CALL _tlgWrapAuto(std::basic_string_view<TChar, TTraits> const& value)


we probably don't need TLG_INLINE either.

jonwis added 2 commits December 29, 2025 15:28

Add support for tracelogging of string types

0ed34ae

Merge remote-tracking branch 'origin/master' into user/jonwis/tracelo…

6ab48c8

…gging-for-strings

jonwis requested review from ChrisGuzak, dunhor, idigdoug and keith-horton December 30, 2025 07:23

jonwis mentioned this pull request Dec 30, 2025

Enable using string views with wil::str_concat #590

Open

jonwis changed the title ~~User/jonwis/tracelogging for strings~~ Enable string-views for use with Tracelogging Dec 30, 2025

idigdoug reviewed Dec 31, 2025

View reviewed changes

dunhor reviewed Jan 4, 2026

View reviewed changes

keith-horton reviewed Jan 5, 2026

View reviewed changes

	if not %RUN_ASAN_TEST%=='' set RUN_ASAN_TEST=1
	if "%RUN_ASAN_TEST%"=="" set RUN_ASAN_TEST=1


		TContainer const& view;

		__forceinline explicit _tlgWrapBufferStlContainer(_In_ TContainer const& ref) : view(ref)

Enable string-views for use with Tracelogging #589

Are you sure you want to change the base?

Enable string-views for use with Tracelogging #589

Conversation

jonwis commented Dec 30, 2025

Uh oh!

jonwis commented Dec 30, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants