Skip to content

Natvis: implement view() specifier, IncludeView/ExcludeView filtering, and CustomListItems loop engine#1580

Open
lugerard wants to merge 19 commits into
microsoft:mainfrom
lugerard:natvis-view-customlistitems
Open

Natvis: implement view() specifier, IncludeView/ExcludeView filtering, and CustomListItems loop engine#1580
lugerard wants to merge 19 commits into
microsoft:mainfrom
lugerard:natvis-view-customlistitems

Conversation

@lugerard

Copy link
Copy Markdown
Contributor

This PR enables MIEngine to evaluate two natvis features:
view-based filtering (IncludeView/ExcludeView on DisplayString
and Expand elements, activated via the view() format specifier) and
<CustomListItems> loop bodies (<Loop>, <Item>, <Exec>,
<If>/<ElseIf>/<Else>, <Break>), which are the standard way to walk
pointer-based data structures such as linked lists and trees.

The two commits are bundled together because <CustomListItems> loop bodies
can contain IncludeView/ExcludeView guards on their <Item> elements,
so commit 1 is a functional prerequisite for commit 2.

Commit 1 adds view() specifier parsing and IncludeView/ExcludeView
filtering, plus fixes to EvalCondition error handling and expression
pre-processing that are prerequisites for correct evaluation on GDB/LLDB.

Commit 2 implements the <CustomListItems> loop engine, including
expression-growth prevention after <Exec> assignments and a $i
word-boundary fix.

All new code is covered by unit tests.

lugerard and others added 3 commits June 10, 2026 14:38
Add view support to the MIEngine natvis evaluator (steps 1, 2, and 3):

1. DisplayString IncludeView/ExcludeView filtering
   - Add IncludeView and ExcludeView properties to DisplayStringType
     (NatvisXsdTypes.cs) so the XML attributes are deserialized.
   - Filter DisplayString entries in FormatDisplayString() using the
     new IsIncludeViewMatch() and IsExcludeViewMatch() helpers before
     evaluating the Condition.

2. {expr,view(name)} inline specifier in DisplayString text
   - ExtractViewName() detects a view() format specifier in an inline
     expression block such as {this,view(RecZone)na}.
   - When detected, FormatValue() calls GetExpressionValue() with the
     view name, which re-enters FormatDisplayString() selecting only
     DisplayString entries whose IncludeView matches.
   - ExtractViewName() returns null for view() with an empty name
     (not a valid specifier).

3. View support on Expand elements
   - Add IncludeView and ExcludeView properties to ExpandedItemType,
     ItemType, and CustomListItemsType in NatvisXsdTypes.cs.
   - Thread currentView through Expand() and ExpandVisualized() so
     that view context propagates into recursive expand calls.
   - Filter Item and ExpandedItem elements by IncludeView/ExcludeView
     before evaluating their Condition.
   - Strip a view() specifier from an ExpandedItem expression before
     evaluating it, and pass the extracted view name into the recursive
     Expand() call so that IncludeView guards on the target type's
     Expand elements match correctly.
   - Add a CustomListItemsType stub case that applies IncludeView/
     ExcludeView filtering; loop body execution to follow.

4. EvalCondition fallback on failure
   - Wrap condition evaluation in try/catch: MIException (debugger
     rejected the expression, e.g. too long) is caught silently and
     returns false; other exceptions are caught and logged at Warning
     before also returning false, so natvis never surfaces errors as
     debug-session failures.
   - Prerequisite: natvis files may use a lightweight condition as a
     platform probe (always true for valid data, but too long to expand
     on GDB/LLDB); without this fallback the condition would surface as
     an error rather than falling through to the next DisplayString.

5. Strip format specifier before name substitution in GetExpressionValue()
   - A specifier such as ",d" was being matched by ProcessNamesInString
     as a child-variable name, corrupting the expression. Strip it
     first, then re-attach after substitution.
   - Prerequisite: without this fix, an expression such as {call(),d}
     would have its specifier matched as a variable name, corrupting
     the expression before evaluation.

6. Intrinsic expansion before dll! stripping in ReplaceNamesInExpression()
   - Intrinsic bodies can contain dll!-qualified type casts. Moving
     expansion before the dll!-strip regex ensures those references are
     also cleaned up.
   - Prerequisite: without this fix, a dll!-qualified cast inside an
     intrinsic body would survive into the expression sent to GDB/LLDB.

Note: items 4, 5, and 6 are bugfixes that are prerequisites for the
view() feature to work correctly on GDB/LLDB. They are bundled in this
commit because they share the same test infrastructure and were
discovered during view() development.

Unit tests:
   Add 18 tests in NatvisFormatSpecifierTest covering the three new
   static helpers (IsIncludeViewMatch, IsExcludeViewMatch, ExtractViewName)
   and the view() specifier parsing patterns used by Expand elements,
   including the empty-name edge case for ExtractViewName.
Add full execution support for the <CustomListItems> expand element,
which was previously deserialized but produced no children.

NatvisXsdTypes.cs:
- Add seven new types covering the loop body:
  CustomListLoopType (<Loop>), CustomListLoopItemType (<Item>),
  CustomListBreakType (<Break>), CustomListExecType (<Exec>),
  CustomListIfType (<If>), CustomListElseIfType (<ElseIf>),
  CustomListElseType (<Else>).
  If/ElseIf/Else bodies and Loop bodies share the same element set,
  so nested loops and conditional branches are fully supported.
- Extend CustomListItemsType with a Loop[] field.
- Add Condition attribute to CustomListLoopType: acts as a while-guard,
  stopping the loop as soon as the condition evaluates to false.
- Add missing [GeneratedCode], [Serializable] and [DebuggerStepThrough]
  attributes to the seven new types for consistency with the rest of
  the file.

Natvis.cs:
- Replace the view-filter stub with a complete execution engine:
  * <Variable> declarations initialise a local-variable table
    (name -> current expression string) via ReplaceNamesInExpression.
  * Optional <Size> sets the totalSize upper bound for pagination.
  * The loop driver evaluates an optional Loop Condition before each
    iteration (while-guard), then calls ExecuteCustomListBody() once
    per iteration.
  * <Break>: stops the loop when the condition holds.
  * <Item>: substitutes local vars, resolves field names and intrinsics,
    emits the child; increments the $i counter (GlobalIndex).
  * <Exec>: supports "varName = rhs" assignment and ++/-- shorthand
    (prefix and postfix) to update the local-variable table.
    After each update, the new expression is evaluated and normalised
    to a scalar literal to prevent unbounded expression growth across
    iterations (e.g. i++ stays "1", "2", "3" rather than accumulating
    nested parentheses).
  * <If>/<ElseIf>/<Else>: evaluates conditions in order and executes
    the first matching branch; all siblings are consumed in one pass
    so the idx cursor never re-processes them.
  * Pagination: fast-forwards through startIndex items, then collects
    up to MAX_EXPAND children; adds a [More...] node when the page is
    full and more items remain.
  * Infinite-loop guard: caps iterations at min(startIndex+51, 10000).
- Add helpers (internal static for testability): ExecuteCustomListBody,
  SubstituteLocalVars, ApplyExecToLocalVars, FormatCustomListItemName
  ($i / {$i} in names with word-boundary guard to avoid corrupting
  tokens like $item), CustomListLoopContext (shared mutable loop
  state), s_execAssignment static Regex (with =(?!=) to avoid matching
  == operators), s_execIncrDecr static Regex for ++/-- shorthand,
  s_dollarI static Regex (\$i\b) for safe bare-$i replacement.

NatvisFormatSpecifierTest.cs:
- 25 new unit tests covering SubstituteLocalVars (empty, no-vars,
  single substitution, word-boundary, multi-var, parens),
  ApplyExecToLocalVars (assignment, unknown LHS, empty, counter
  increment, == not matched, prefix/postfix ++/--, unknown var with
  ++, whitespace tolerance), and FormatCustomListItemName (null
  template, {$i}, bare $i, no special tokens, local-var substitution).

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends MIEngine’s Natvis evaluator to support view-based formatting/expansion (view() specifier plus IncludeView/ExcludeView filtering) and adds a CustomListItems loop execution engine to enable pointer-based collection traversal during expansion.

Changes:

  • Adds parsing/propagation of view(name) and applies IncludeView/ExcludeView filtering for DisplayString, Item, ExpandedItem, and CustomListItems.
  • Improves Natvis expression handling (specifier stripping before name substitution, intrinsic/module-prefix preprocessing ordering, and safer EvalCondition failure handling).
  • Implements CustomListItems loop-body execution (Loop/Item/Exec/If/ElseIf/Else/Break) with helper utilities and unit tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
src/MIDebugEngineUnitTests/NatvisFormatSpecifierTest.cs Adds unit tests for view() extraction, view matching helpers, and CustomListItems helper utilities.
src/MIDebugEngine/Natvis.Impl/NatvisXsdTypes.cs Extends Natvis XSD-derived types with view attributes and new CustomListItems loop-body element models.
src/MIDebugEngine/Natvis.Impl/Natvis.cs Implements view filtering, view() propagation through expansion/formatting, expression preprocessing fixes, and the CustomListItems loop engine.

Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs
Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs Outdated
Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs Outdated
Comment thread src/MIDebugEngine/Natvis.Impl/NatvisXsdTypes.cs
Comment thread src/MIDebugEngine/Natvis.Impl/NatvisXsdTypes.cs Outdated
Comment thread src/MIDebugEngine/Natvis.Impl/NatvisXsdTypes.cs Outdated
Comment thread src/MIDebugEngine/Natvis.Impl/NatvisXsdTypes.cs Outdated
@gregg-miskelly

Copy link
Copy Markdown
Member

Can you add an end-to-end test that uses this? Example: #1559

Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs
Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs Outdated
Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs Outdated
Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs

@gregg-miskelly gregg-miskelly left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this. Aside from that, this seems about right to me.

lugerard and others added 11 commits July 2, 2026 15:08
Rename the <CustomListItems> loop-body types to the names used in the main
natvis.xsd, and fix the <Elseif> element casing:
  CustomListLoopItemType -> CustomListItemType
  CustomListBreakType    -> BreakType
  CustomListExecType     -> ExecType
  CustomListIfType       -> IfType
  CustomListElseType     -> ElseType
  CustomListLoopType     -> LoopType
  element name "ElseIf"  -> "Elseif"

IfType change: the xsd declares <Elseif type="IfType">, so it shares IfType with
<If> instead of having its own type. A single object[] member cannot map two
element names ("If" and "Elseif") to the same type; XmlSerializer throws at
construction. The generated form of the schema solves this with an
XmlChoiceIdentifier, so this commit does the same:
  - <Elseif> reuses IfType (the separate CustomListElseIfType is removed).
  - IfType/ElseType/LoopType gain a parallel ItemsChoiceType[] ItemsElementName
    array, declared via [XmlChoiceIdentifier], that records which element name
    produced each Items entry.
  - ExecuteCustomListBody now receives that choice array and uses it to tell an
    <If> from an <Elseif>, since both deserialize to IfType.

This is what regenerating NatvisXsdTypes.cs from natvis.xsd would produce,
keeping the hand-maintained file consistent with the schema.
…d <Exec> normalization

Two robustness fixes in the CustomListItems loop engine:

- ExecuteCustomListBody now ends with an else branch that logs a warning naming
  any unrecognised loop-body element, instead of silently skipping it.
- The <Exec> normalization catch now logs a warning and stops the loop.

We log and break in the second case because the normalization evaluates the loop
variable we just updated; the loop body and its conditions use that same
variable, so if it can't be evaluated the iteration can't continue meaningfully.
Rather than press on with a variable we know is broken (which would emit error
items or run to the iteration cap) we log the failure and stop the loop.
.natvis files shipped with VS often assign to several variables in one <Exec>
block (e.g. "<Exec>++idx, ++statptr</Exec>"), which we previously did not handle.

- Multi-variable support: ApplyExecToLocalVars splits the <Exec> text on
  top-level commas (commas inside parentheses/brackets are left intact, so
  "f(a, b)" is not split) and applies each assignment in turn, returning all
  updated variable names so each is normalised.
- Warning: a non-empty segment we cannot apply (an unsupported expression such
  as "idx += 2" or a call, or an undeclared left-hand side) is now logged at
  Warning instead of being silently dropped.
- Fix: the <Exec> value was being substituted by SubstituteLocalVars *before*
  ApplyExecToLocalVars parsed it, turning "i = i + 1" into "(0) = (0) + 1" so the
  "(\w+)" left-hand-side match failed and the assignment was silently dropped; no
  <Exec> assignment took effect at runtime (the unit tests passed only because
  they call the helper with raw text). The helper is now given the raw <Exec>
  text and does the right-hand-side substitution itself.

Tests cover multiple increments, multiple assignments in order, the returned
name list, a comma inside parentheses, a trailing empty segment, unhandled
segments reported for an unsupported form and an undeclared left-hand side, and
SplitTopLevelCommas.
The top-level loop driver (in ExpandVisualized) and the nested <Loop> branch in
ExecuteCustomListBody were near-identical copies of the same iteration logic: the
iteration cap, the optional while-Condition guard, the per-pass
ExecuteCustomListBody call, and the no-progress break. Extract them into a single
DriveLoop helper used by both. No behaviour change.
- Sequential <Variable> init
- Pointer-safe <Exec> normalization

Adds unit tests for IsScalarLiteral.
CppTests integration test exercising the <CustomListItems> loop engine

- CustomListContainer.h: a NULL-terminated singly-linked list
- main.cpp: a customList(10, 20, 30) before the return breakpoint
- Simple.natvis: a <CustomListItems> visualizer (Variable/Loop/Break/Item/Exec)
- NatvisTests.cs: TestCustomListItems asserts Count=3 and [0..2]=10/20/30,
  and bumps the breakpoint line constants for the grown main.cpp
CppTests integration test for IncludeView/ExcludeView and a ",view(name)"
specifier invoked from inside the natvis:

- main.cpp: a ViewObject{x, y, z} and a ViewHolder wrapping one, before the
  return breakpoint
- Simple.natvis: ViewObject tags a DisplayString and Expand items with
  IncludeView/ExcludeView; ViewHolder invokes the view via
  {inner,view(simple)} and <ExpandedItem>inner,view(simple)</ExpandedItem>
- NatvisTests.cs: TestViewFiltering asserts the default view (tagged
  DisplayString and Item skipped) and the simple view through ViewHolder
  (IncludeView DisplayString selected, view propagated into the expansion,
  ExcludeView item filtered out)
A watch expression like "obj,view(simple)" previously reached the debugger
unchanged and failed to evaluate (lldb: "use of undeclared identifier"). The
view() specifier is a natvis concept, so the debugger should never see it:

- VariableInformation.ProcessFormatSpecifiers now recognizes a view()
  specifier, strips it from the evaluated expression, and records the view
  name. It is handled before the modifier stripping, which could otherwise
  corrupt a view name containing a modifier letter pair (e.g. "view(second)").
- IVariableInformation exposes the recorded view, and AD7Property passes it
  to FormatDisplayString and Expand, so the named view selects the
  DisplayString and filters IncludeView/ExcludeView items in the expansion.

The test harness could so far only read the one-line result of a watch
expression, not what it expands to: the debugger's reply includes a handle
for expanding the result (variablesReference), but the harness discarded it.
It now keeps the handle, and the new FrameInspector.EvaluateChildren uses it
to return the child rows of an evaluated expression. TestViewFiltering uses
both: "vo,view(simple)" must summarize as the IncludeView DisplayString and
expand to the view-filtered items.
@lugerard

lugerard commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Can you add an end-to-end test that uses this? Example: #1559

done

Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs Outdated
{
children.Add(new PaginatedVisualizerWrapper(
ResourceStrings.MoreView, _process.Engine, variable,
visualizer, isVisualizerView: true, ctx.StartIndex + MAX_EXPAND));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not quite understanding the "more elements" behavior here. Don't we need to save state from the expansion so that we can keep going where we left off?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The [More...] node only stored the next start index: expanding it re-ran the loop from the beginning and skipped the items already shown, so each page re-walked everything before it. It now saves the state instead, as you suggest: the loop variables (compact literals after the fix from your other comment) and the position, and expansion resumes from there, like the LinkedListItems/TreeItems continuations. Pages of the 500-element list now take a flat ~0.13s each regardless of depth.

Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs Outdated
// A segment we could not apply (unsupported form, or an undeclared
// left-hand side) would otherwise silently do nothing; log it so the
// omission is visible rather than producing a wrong/stuck traversal.
_process.Logger.NatvisLogger?.WriteLine(LogLevel.Warning, "CustomListItems <Exec> segment not applied (unsupported expression or undeclared variable): " + seg);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_process

Stop looping?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs Outdated
// iteration starts compact. Anything else must keep its expression:
// GetExpressionValue runs FormatDisplayString, so a pointer/struct local
// is rendered as "0x..." or a visualizer display string — storing that
// (a type-less address or non-C++ text) would corrupt later substitutions.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels like it isn't going to scale past really tiny collections. Can you do some perf testing with a link list of say 500 elements?

Assuming this is a problem, my thought:

  • Just handle integer types, and pointers, for pointers, we can save the hex address and cast it to the "type" value (ex: (MyStruct*)0x1234)
  • For anything else: maybe add an error row after the first few elements

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed with a 500-element linked list (lldb): each page was slower than the last (0.17s → 0.90s over the first five) and page 6 hung outright once the expression passed ~250 nested levels. Implemented your suggestion. The full list now expands in ~6s (10 pages, longest 1.1s). The residual per-page growth is the [More...] re-walk from your other comment.

lugerard added 3 commits July 7, 2026 10:57
…e applied

An unapplied segment means the loop state did not advance the way the
visualizer intended; continuing emits wrong items or runs to the iteration
cap. Previously the segment was logged and the loop kept going.
…lists evaluable

Each <Exec> step grew the updated variable's stored expression by one level
("((node)->next)->next..."), so evaluating the nth element cost O(n) and
walking a list O(n^2). Past roughly 250 levels the expression exceeded
lldb's limits: expanding a 500-element list hung partway through and never
completed.

The variable is now evaluated raw (no natvis formatting) after each <Exec>
and stored back as a constant-size expression when possible:
- integers and bools as literals
- pointers as the address cast to their type ("(MyStruct *)0x1234"), the
  same simplification VariableInformation.FullName applies to long parent
  expressions; the address is stable while the process is stopped
- anything else keeps its expression; after a few growing steps an error row
  is emitted and the loop stops rather than degrade item by item

Adds MakeCompactLiteral unit tests, a 500-element list to the natvis
debuggee, and paging assertions to TestCustomListItems.
Expanding a "[More...]" node re-ran the loop from the start and skipped the
already-shown items, so each page re-walked everything before it.

The page is now cut at a pass boundary and the "[More...]" node saves the
loop state there (the local-variable table, compact after <Exec>
normalization, and the loop position); expanding it resumes the walk in
place, like the LinkedListItems and TreeItems continuations. A pass always
runs to completion so none of its <Item>s are lost, which can let a page
slightly exceed the page size.
Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs Outdated
{
string szExpr = SubstituteLocalVars(sz.Value?.Trim() ?? "0", localVars);
string szVal = GetExpressionValue(szExpr, variable, visualizer.ScopedNames, visualizer.Intrinsics);
totalSize = MICore.Debugger.ParseUint(szVal, throwOnError: false);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

false

This means that the size will be zero if the value isn't a uint. Is that what you want?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. Fixed so it falls back to Break-driven termination instead of an empty expansion.

Comment thread src/MIDebugEngine/Natvis.Impl/Natvis.cs Outdated
internal static bool IsScalarLiteral(string value)
{
return !string.IsNullOrEmpty(value) && s_integerLiteral.IsMatch(value);
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method doesn't seem to be used aside from unit testing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed. Leftover after MakeCompactLiteral replaced it. Removed, along with its unit tests.

// degrade item by item.
try
{
IVariableInformation updated = GetExpression(localVars[updatedVar], variable, visualizer.ScopedNames, null, visualizer.Intrinsics);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GetExpression

MakeCompactLiteral depends on integers coming back in decimal instead of hex, but VaribaleInfo.SyncEval is going to use the engine's current radix. So you need to either change MakeCompactLiteral to be less picky, or you need GetExpression to take an optional radix argument and push that though

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made MakeCompactLiteral less picky: hex integers are stored back with their type cast ((int)0xffffffd6), which also keeps negative values negative; a bare hex literal is unsigned. Pointers are recognized by their type name first. Verified paging of the 500-element list under hex radix; the integer-literal cases are unit-tested.

lugerard added 2 commits July 10, 2026 12:51
…arsed

ParseUint(throwOnError: false) returns zero on unparsable input without
throwing, so the intended fallback in the catch block (totalSize stays
MaxValue and <Break> terminates the loop) never applied: an unparsable
<Size> produced an empty expansion instead.
With the engine radix set to 16, integers evaluate to hex text ("0x1f"),
which the decimal-only literal check rejected: integer loop variables
stopped compacting and the loop ended with the error row. Hex integers are
now stored back with their type cast ("(int)0xffffffd6"), which a bare hex
literal would not survive: it is unsigned, so a negative value would flip
sign in later comparisons. Pointers are recognized by type first, since a
pointer must keep its type cast either way.

Removes IsScalarLiteral, no longer used outside its unit tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants