Implement natvis 'na' modifier and $T substitution - #1612
Conversation
Add support for the natvis 'na' (no-address) modifier to strip MI's leading address prefix from string values. This requires tracking the format specifier through variable initialization and applying cleanup at value retrieval points. Also implement template parameter substitution ($T1, $T2, etc.) within natvis brace expressions before format specifier extraction.
There was a problem hiding this comment.
Pull request overview
This PR extends MIEngine’s NatVis evaluator to (1) support the na (no-address) modifier by stripping MI’s leading 0x... address prefix from certain evaluated values, and (2) perform $T1, $T2, … template-parameter substitution inside NatVis brace expressions before parsing format specifiers.
Changes:
- Add
$Tnmacro substitution over full{...}brace expressions in Natvis display-string formatting. - Detect and apply
naaddress-prefix stripping during NatVis display-string formatting and during variable evaluation/initialization by tracking an_formatHasNaflag. - Update NatVis string “cleanup” helpers and related comments around address-prefix stripping behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/MIDebugEngine/Natvis.Impl/Natvis.cs | Adds $Tn substitution in brace expressions and introduces na detection/cleanup in display-string formatting; updates format-specifier helpers and string cleanup docs. |
| src/MIDebugEngine/Engine.Impl/Variables.cs | Tracks na in parsed format specifiers and attempts to strip MI’s address prefix at variable construction/evaluation time. |
| return mt.Value; | ||
| }); | ||
| } | ||
| bool hasNa = HasNaModifier(rawExpr); |
| if (spec == "sub" || spec == "su") | ||
| exprValue = CleanUtf16StringValue(exprValue); | ||
| else if (spec == "sb") | ||
| exprValue = CleanAsciiStringValue(exprValue); |
There was a problem hiding this comment.
Did you mean to loose this sub/su/sb code?
There was a problem hiding this comment.
Actually, is su needed? Variabel.cs line 477 already handles su correctly.
There was a problem hiding this comment.
This problem seems to still be here
| value = value.EndsWith("\"", StringComparison.Ordinal) | ||
| ? value.Substring(2, value.Length - 3) | ||
| : value.Substring(2); | ||
| } |
There was a problem hiding this comment.
,sub is supposed to remove the quotes/prefix
There was a problem hiding this comment.
Though this changes looks like it is correct for ,su
| return replacement; | ||
| return mt.Value; | ||
| }); | ||
| } |
There was a problem hiding this comment.
If I am reading the code correctly, this is already at least partially done by ReplaceNamesInExpression, which is called by GetExpressionValue, so I don't think you want to do this over the entire expression.
Can you trace through why ReplaceNamesInExpression isn't doing this already? (lines 1713-1717).
There was a problem hiding this comment.
Yeah it seems like ReplaceNamesInExpression should properly handle the substitution, and it seems like it gets called at the right time, so I have no answer for why it isn't doing it already
There was a problem hiding this comment.
We should debug through it and see why it doesn't work. Let me know if you need help.
|
Can you add a test for your changes? You may also need to adjust a test baseline for |
The test seems to get properly handled via TestDisplayString in NatvisTests to me |
| TypeName = results.TryFindString("type"); | ||
| Value = results.TryFindString("value"); | ||
| // Only strip the leading MI address prefix ("0x... ") when the natvis format included the 'na' modifier. | ||
| Value = StripLeadingAddress(Value, _formatHasNa); |
| if (hasNa && !string.IsNullOrEmpty(exprValue)) | ||
| { | ||
| exprValue = s_addressPrefix.Replace(exprValue, ""); | ||
| } |
There was a problem hiding this comment.
Suggestion: Make VariableInformation.StripLeadingAddress public so you can use it here
| /// "nr", "nd" are stripped before returning. Returns null when no specifier is present. | ||
| /// </summary> | ||
| internal static string ExtractFormatSpecifier(string expression) | ||
| internal static string ExtractFormatSpecifier(string expression, out bool hasNa) |
| TypeName = results.TryFindString("type"); | ||
| Value = results.TryFindString("value"); | ||
| // Only strip the leading MI address prefix ("0x... ") when the natvis format included the 'na' modifier. | ||
| Value = StripLeadingAddress(Value, _formatHasNa); |
match tests with out param from extractformatspecifier made VariableInformation.StripLeadingAddress internal static to make public and updated natvis.cs accordingly changed _formathasna to inherit properly and call stripleadingaddress more strictly
| string expFS = exp.Substring(lastComma + 1).Trim(); | ||
| // Detect whether the natvis 'na' modifier is present in the original format specifier. | ||
| // We must detect this before we strip modifiers below. | ||
| _formatHasNa = expFS.IndexOf("na", StringComparison.Ordinal) >= 0; |
There was a problem hiding this comment.
| string tail = expression.Substring(commaPos + 1); | ||
| return tail.IndexOf("na", StringComparison.Ordinal) >= 0; | ||
| } | ||
|
|
There was a problem hiding this comment.
This method should be deleted in favor of just using ExtractFormatSpecifier
Add support for the natvis 'na' (no-address) modifier to strip MI's leading address prefix from string values. This requires tracking the format specifier through variable initialization and applying cleanup at value retrieval points.
Also implement template parameter substitution ($T1, $T2, etc.) within natvis brace expressions before format specifier extraction.