<xmemory>: Document the iterator debugging invariants

[Prakriti Sharma (prakriti31)]

Closes #2084.

The iterator debugging (IDL) proxy/iterator machinery in <xmemory>
(_Container_proxy, _Container_base12, _Iterator_base12) wasn't
commented at all, even though its invariants are non-obvious (intrusive
linked list of iterators, mutual container/proxy pointers, locking rules).
Stephan T. Lavavej (@StephanTLavavej) explained these invariants in the issue thread (originally
via a linked Discord screenshot from 2021), so this PR just captures that
explanation as a comment next to the code it describes, so it doesn't only
live in a screenshot linked from a 4-year-old issue comment.

I verified the described invariants (mutual _Myproxy/_Mycont pointers, the
_Myfirstiter/_Mynextiter intrusive list, the _Lockit(_LOCK_DEBUG) locking
rule) still match the current code exactly before writing this up.

No behavior change -- comment-only.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[Copilot]

Pull request overview

Documents iterator-debugging proxy relationships, iterator lists, and locking behavior in <xmemory>.

Changes:

• Adds comments describing iterator-debugging invariants.
• Introduces no behavioral changes.

Suppressed comments (2)

stl/inc/xmemory:1222

• This invariant only holds when _ITERATOR_DEBUG_LEVEL == 2. At level 1, _Iterator_base12::_Adopt (lines 1327–1333) copies _Myproxy but never links the iterator into _Myfirstiter, so valid iterators are deliberately absent from this list. Please qualify the invariant so the documentation matches both _Container_base12 modes.

// * The proxy's _Myfirstiter, together with each iterator's _Mynextiter, forms an intrusive singly linked list of
//   iterators rooted at the proxy. Every valid iterator belonging to a container is reachable through this list;
//   there are no valid "free-floating" iterators.

stl/inc/xmemory:1226

• The lock rule is not unconditional: level-1 proxy swaps use the unlocked helper (lines 1457–1459), and level-2 constant evaluation also invokes unlocked helpers. Runtime list mutations at level 2 are locked, but proxy allocation/reloading/deletion is not. Please state that narrower invariant instead of saying every proxy manipulation holds _LOCK_DEBUG.

// * Whenever the proxies and the intrusive list are manipulated, the debug lock (_Lockit(_LOCK_DEBUG)) is held.
//   The only things we do outside of that lock are things like iterator compatibility checks that compare proxy
//   pointers: those pointers don't change even if the containers are being swapped concurrently (only the
//   proxies' data members change, not their addresses).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Prakriti Sharma (prakriti31)]

@microsoft-github-policy-service agree

[Prakriti Sharma (prakriti31)]

Stephan T. Lavavej (@StephanTLavavej) What else do we need to merge it. Can you help me? I will push the changes asap.

[Stephan T. Lavavej (StephanTLavavej)]

Someone who is deeply familiar with the invariants (i.e. me) needs to review this and think about whether the description is correct and complete.

I have a high-priority task to get through (constexpr <cmath>) and then I'm going on vacation, but I'll get to this when I return and start dealing with the PR backlog. Apologies for the delay.

[Prakriti Sharma (prakriti31)]

Sure!!

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (6)

stl/inc/xmemory:1232

• The claim about containers being swapped concurrently reads like a thread-safety guarantee, but swap racing with iterator operations is still undefined behavior and can involve data races even if proxy addresses are stable. Consider rewording to avoid implying concurrency safety (e.g., focus on the intended invariant: proxy objects aren’t relocated, so address comparisons remain meaningful without taking the debug lock).

//   isn't usable there. The only other things we do outside of the lock at runtime are things like iterator
//   compatibility checks that compare proxy pointers: those pointers don't change even if the containers are
//   being swapped concurrently (only the proxies' data members change, not their addresses).

stl/inc/xmemory:1215

• This bullet is phrased as applying to 'Every container', but the machinery here is specifically about containers participating in IDL (i.e., those using _Container_base12 / the proxy mechanism). To prevent readers from overgeneralizing, consider scoping the statement (e.g., 'Every IDL-enabled container' or 'Every container derived from _Container_base12').

// * Every container owns a dynamically allocated _Container_proxy at all times, including in its

stl/inc/xmemory:1216

• The inline 'This is TRANSITION, ABI:' reads awkwardly and is inconsistent with typical tag-style notes. Consider restructuring this into a standalone // TRANSITION, ABI: line (or rephrasing the sentence) so the rationale is clearer and the tag is easier to scan.

//   default-constructed and moved-from states. This is TRANSITION, ABI: allocating the proxy separately (instead

stl/inc/xmemory:1211

• Several of these comment lines are quite long. If this repo follows a column limit for headers, wrapping these lines would improve readability and keep formatting consistent with surrounding code/comments.

// The machinery below implements iterator debugging (informally "IDL", after the _ITERATOR_DEBUG_LEVEL macro,

stl/inc/xmemory:1217

• Several of these comment lines are quite long. If this repo follows a column limit for headers, wrapping these lines would improve readability and keep formatting consistent with surrounding code/comments.

//   of, say, storing it inline) is the major reason many containers' allocator-extended move constructors and

stl/inc/xmemory:1227

• Several of these comment lines are quite long. If this repo follows a column limit for headers, wrapping these lines would improve readability and keep formatting consistent with surrounding code/comments.

// * The proxy's _Myfirstiter, together with each iterator's _Mynextiter, forms an intrusive singly linked list of
//   iterators rooted at the proxy. Every valid iterator belonging to a container is reachable through this list;
//   there are no valid "free-floating" iterators.
// * Whenever the proxies and the intrusive list are manipulated at runtime, the debug lock (_Lockit(_LOCK_DEBUG))

[Copilot]

🟡 Changes recommended

The documented invariants need correction before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

stl/inc/xmemory:1228

• The list and locking invariants apply only at IDL 2. At IDL 1, _Adopt merely copies _Myproxy (xmemory:1332-1339) and proxy swapping takes the unlocked branch (xmemory:1463-1465); _Alloc_proxy and _Reload_proxy also mutate proxy state without locking. Please qualify list membership to IDL 2 and narrow the lock claim to the operations that actually take it.

// * The proxy's _Myfirstiter, together with each iterator's _Mynextiter, forms an intrusive singly linked list of
//   iterators rooted at the proxy. Every valid iterator belonging to a container is reachable through this list;
//   there are no valid "free-floating" iterators.
// * Whenever the proxies and the intrusive list are manipulated at runtime, the debug lock (_Lockit(_LOCK_DEBUG))
//   is held. During constant evaluation, we skip the lock entirely and go straight to the unlocked paths (see the

stl/inc/xmemory:1221

• The “always”/“regardless” claim is inaccurate at level 0, where _Container_base0 has no _Myproxy and no proxy is allocated. It also overlooks intentionally unbound proxies created by _Container_proxy_ptr12(..., _Leave_proxy_unbound), so state this as an invariant of bound proxies when IDL is enabled.

// * A container and its proxy always point to each other (_Container_base12::_Myproxy and
//   _Container_proxy::_Mycont, respectively), regardless of whether IDL is enabled; if a proxy exists, this holds.

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The unresolved deque documentation accuracy concern warrants final human review.

Review details

Suppressed comments (1)

stl/inc/xmemory:1215

• This overlooks deque: _Deque_val deliberately derives from _Container_base12 unconditionally and deque allocates _Container_proxy objects even when _ITERATOR_DEBUG_LEVEL == 0 (stl/inc/deque:533 and stl/inc/deque:643-645). As written, the new overview gives an incorrect level-0 invariant; please document the ABI-preserving exception.

// needing to know about the other directly, by routing everything through a shared proxy object. Only containers
// with _ITERATOR_DEBUG_LEVEL != 0 use it; at level 0 they derive from _Container_base0 and use
// _Fake_proxy_ptr_impl instead, and no proxy is allocated at all.

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The only unresolved comment is a minor documentation clarification that does not block approval.

Review details

Suppressed comments (1)

stl/inc/xmemory:1248

• Clarify which pointers remain stable during a swap. _Swap_proxy_and_iterators_unlocked swaps each container's _Myproxy value (xmemory:1455-1458), so the current broad statement that “proxy pointers” do not change can be read as contradicting the implementation. It is specifically existing iterators' _Myproxy values and the proxy object addresses that remain unchanged; each proxy's _Mycont is then updated.

// directly. The other things we do outside the lock are things like iterator compatibility checks that compare
// proxy pointers: those pointers don't change when containers are swapped (only the proxies' data members
// change, not their addresses).

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The documented invariants contain three inaccuracies that should be corrected.

Review details

Suppressed comments (3)

stl/inc/xmemory:1232

• This invariant is not true below level 2. At levels 0 and 1, _Container_base12::_Orphan_all() is a no-op, so an invalidated iterator can retain a non-null (and after destruction, dangling) _Myproxy. Since the introduction uses “orphan” as a synonym for invalidation, please restrict the nulling guarantee to level 2.

// * Every valid iterator holds a non-owning pointer to its parent container's proxy (_Iterator_base12::_Myproxy);
//   an orphaned iterator has a null _Myproxy.

stl/inc/xmemory:1248

• This conflates three different pointers. Compatibility checks compare container addresses returned by _Getcont() (for example, deque:388), while _Swap_proxy_and_iterators_unlocked() exchanges the containers' _Myproxy values and rewrites each proxy's _Mycont; only existing iterators' _Myproxy values remain unchanged. Please state that distinction explicitly so the documented swap invariant matches the implementation.

// directly. The other things we do outside the lock are things like iterator compatibility checks that compare
// proxy pointers: those pointers don't change when containers are swapped (only the proxies' data members
// change, not their addresses).

stl/inc/xmemory:1227

• The noexcept rationale does not match the implementations. For example, vector(vector&&, const Alloc&) allocates its proxy even on its noexcept(is_always_equal) path (vector:770-777), while the _Reload_proxy branch in move assignment is explicitly inside a noexcept path and terminates on allocation failure (vector:800-824). The potentially throwing unequal, non-propagating path moves elements and does not reload the proxy. Please describe the ABI/allocation constraint without claiming that proxy reload determines these noexcept specifications.

//   default-constructed and moved-from states. This is TRANSITION, ABI: allocating the proxy separately (instead
//   of, say, storing it inline) is the major reason many containers' allocator-extended move constructors and
//   move assignment operators aren't unconditionally noexcept -- reloading the proxy when allocators compare
//   unequal can throw. We intend to revisit this strategy in vNext (see GH-169).

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

Correct the inaccurate _Myproxy invalidation invariant before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟢 Approval recommended

The comment-only change is safe, with only a non-blocking line-ending nit remaining.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The malformed and duplicated comment text should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The documentation block is corrupted and redundant, requiring one clean replacement paragraph.

Review details

Suppressed comments (1)

stl/inc/xmemory:1237

• The new invariant paragraph contains malformed line endings after the first two continuation lines and then repeats “becomes dangling only when the proxy itself is destroyed.” This leaves the documentation block corrupted/redundant; replace the paragraph with one clean copy.

//   Orphaning an iterator nulls that pointer, but only level 2 tracks invalidation and can null iterators through

//   _Orphan_all. Below it, _Orphan_all is a no-op, so an invalidated iterator retains its _Myproxy; that pointer

//   becomes dangling only when the proxy itself is destroyed.

//   becomes dangling only when the proxy itself is destroyed.

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The orphaning and free-floating iterator claims need qualification.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

stl/inc/xmemory:1241

• “There are no valid free-floating iterators” is not an invariant of all users of this machinery. A default or exhausted regex_iterator is a valid end-of-sequence iterator with no proxy (stl/inc/regex:2965, 3037, and 3058). Please scope the assertion to iterators that are currently associated with a container.

// * The proxy's _Myfirstiter, together with each iterator's _Mynextiter, forms an intrusive singly linked list of
//   iterators rooted at the proxy. Every valid iterator belonging to a container is reachable through this list;
//   there are no valid "free-floating" iterators. Below level 2 the list is unused: _Adopt merely copies
//   _Myproxy, _Orphan_all does nothing, and _Myfirstiter remains null.

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The malformed line endings must be normalized before approval.

Review details

Suppressed comments (1)

stl/inc/xmemory:1235

• These four lines use CRCRLF endings, unlike the surrounding single-CRLF lines (for example, lines 1229–1231). The extra carriage returns appear as embedded control characters and can confuse line-based formatting checks; normalize each ending to one CRLF.

// * Every iterator associated with a parent container holds a non-owning pointer to that container's proxy

//   (_Iterator_base12::_Myproxy). Container-driven orphaning nulls that pointer, but _Orphan_all only does so at

//   level 2. Below level 2, an iterator invalidated by its container retains its _Myproxy, which dangles only once

//   the proxy itself is destroyed.

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[Copilot]

🔵 Needs a closer look

The inconsistent line endings should be corrected before approval.

Review details

Suppressed comments (1)

stl/inc/xmemory:1235

• These four lines use CRCRLF endings while the rest of this file uses CRLF, leaving an extra carriage-return byte on each line (and likely triggering whitespace checks). Please normalize these lines to the file's existing line endings.

// * Every iterator associated with a parent container holds a non-owning pointer to that container's proxy

//   (_Iterator_base12::_Myproxy). Container-driven orphaning nulls that pointer, but _Orphan_all only does so at

//   level 2. Below level 2, an iterator invalidated by its container retains its _Myproxy, which dangles only once

//   the proxy itself is destroyed.

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced