STL.natvis: Simplify visualization for string

[pps83]

It appears that debugger visualizer accepts na regardless if underlying char type

[pps83]

here's debugger view:

    std::string s15 = "012345678901234";
    std::string s16 = "0123456789012345";
    std::string::iterator s15i = s15.begin() + 1;
    std::string::const_iterator s16ic = s16.cbegin() + 1;

    std::wstring w7 = L"0123456";
    std::wstring w8 = L"01234567";
    std::wstring::iterator w7i = w7.begin() + 1;
    std::wstring::const_iterator w8ic = w8.cbegin() + 1;

    std::u32string u3 = U"012";
    std::u32string u4 = U"0123";
    std::u32string::iterator u3i = u3.begin() + 1;
    std::u32string::const_iterator u4ic = u4.cbegin() + 1;

[pps83]

Note, it does break std::basic_string<unsigned short> case (std::basic_string<short> didn't work before or after). With the change applied unsigned short and short versions look consistent at least.

    std::wstring w7 = L"0123456";
    std::wstring w8 = L"01234567";

    std::basic_string<unsigned short> x7 = (unsigned short*)w7.c_str();
    std::basic_string<unsigned short> x8 = (unsigned short*)w8.c_str();
    std::basic_string<unsigned short>::iterator x7i = x7.begin() + 1;
    std::basic_string<unsigned short>::const_iterator x8ic = x8.cbegin() + 1;

[pps83]

This should be final update. Please read this msg with full details: #5176 (comment)

[CaseyCarter]

Wow, that's a lot of cleanup. Very nice! Thanks for all the investigation it took to make sure this was right.

[StephanTLavavej]

Thanks! 😻 I pushed a comment update and two simplifications for wildcards.

⚠️ I'm having trouble getting the VS IDE to pick up the updated natvis (I couldn't get it to display an added "taxicab" node), so I was unable to verify these changes, and I'm totally YOLOing them. Please double-check @CaseyCarter and @pps83 - I apologize if I messed up the PR.

For future reference, after code review begins, please avoid force-pushing changes as that makes it harder to incrementally review what changed.

Update: On Discord, Casey figured out why adding stl.natvis to my project didn't appear to be taking effect:

It looks like the debugger is preferring e.g. the more-specific std::basic_string<char,*> from the installed STL.natvis to our std::basic_string<*>. (Changing "Tools > Options > Debugging > Output Window > Natvis diagnostic messages" to "Verbose" is handy for debugging visualizer weirdness.) I do see other visualizers from the repo activating (Natvis: C:\STL\stl\debugger\STL.natvis(1041,6): Successfully parsed expression '_Mydata' in type context 'std::basic_string_view<char,std::char_traits<char> >'.) so it appears that https://learn.microsoft.com/en-us/visualstudio/debugger/create-custom-views-of-native-objects?view=vs-2022#BKMK_natvis_location is telling the truth about the search order for visualizers within natvis files.
It looks like we'll need to rename the installed STL.natvis with some other extension so it won't be loaded and we can cleanly test the incoming STL.natvis.

I didn't realize that all loaded visualizers were effectively being thrown into a giant pile, and the preference order for file fallbacks was being applied after "what visualizer is more specialized". I thought the file fallbacks were coarse-grained, but the actual behavior makes more sense, even if it's annoying for this development scenario.

This meant that we weren't actually testing anything, both before and after my changes (std::basic_string<*,*> was still less specific than the original std::basic_string<char,*>). Casey re-validated everything with no fallback so we're good now.

[pps83]

I also updated CPPDebuggerVisualizers, it appears boost::string didn't even have correct visualizers. What's interesting, it has InternalBufferChars variable to avoid calculations in string::capacity for short strings. Perhaps, ms/STL has something like that (or at least it should) and avoid doing this in visualizers:

<Intrinsic Name="bufSize" Expression="16 / sizeof($T1) &lt; 1 ? 1 : 16 / sizeof($T1)" />

For future reference, after code review begins, please avoid force-pushing changes as that makes it harder to incrementally review what changed.

Yes, I know that, on my side these two PRs were messed up, I ended up fixing them manually and force pushing final versions. Sorry for that.

[CaseyCarter]

IIRC there was an issue with one of the compilers (clang-cl?) not emitting static constexpr class members into the PDB when we last overhauled std::string so we replicated the calculation in the visualizer as a workaround. It may not be necessary anymore, but nobody has investigated.

[StephanTLavavej]

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

[StephanTLavavej]

Thanks for this dramatic simplification, and congratulations on your second microsoft/STL commit! 🐈 🐈‍⬛ 😻