iterator_base advertises forward_iterator_tag but violates the multi-pass guarantee

Aperta
  • Qualcosa non va?
#1,446 6 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
4/5
Tempo stimato
3-5 giorni
Idoneità per principianti
45/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Tranquilla
Stack tecnologico
cpp
Ambito
backend

Direzione di ricerca

Start with include/yaml-cpp/node/detail/iterator.h, where the issue says iterator_base<V>::operator*() returns value_type by value; inspect the iterator category and its behavior when equal iterators are dereferenced. The reproduction in the issue shows the failing address comparison and a passing std::forward_list control. Done means the iterator's declared category matches its multi-pass behavior, with the reproduction confirming the result.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

Summary

YAML::Node's iterator declares iterator_category = std::forward_iterator_tag but does not satisfy the semantic multi-pass guarantee required by std::forward_iterator (iterator.concept.forward). This causes silent incorrect behaviour when the iterator is used with C++20 <ranges> machinery, which trusts the declared category.

What the standard requires

std::forward_iterator requires the multi-pass guarantee: if a == b, then *a and *b must refer to the same object. This is a semantic requirement; there is no compile-time concept check that enforces it.

std::views::transform and other range adapters propagate the iterator category from the underlying range. If a range claims to be a forward_range, the resulting view also claims to be one. Code downstream that relies on multi-pass behaviour then has formally undefined behaviour.

Reproduction
#include <yaml-cpp/yaml.h>
#include <gtest/gtest.h>
#include <memory>
#include <forward_list>

// Syntactic check passes -- concepts only see iterator_category, not semantics.
static_assert(std::ranges::forward_range<YAML::Node>);

// FAILS at runtime: two copies of the same iterator dereference to distinct objects.
// std::forward_iterator requires *a and *b to refer to the same object when a == b.
// https://en.cppreference.com/w/cpp/iterator/forward_iterator
TEST(YamlCppMapIterator, DereferenceOperator_WithEqualIteratorCopies_ReturnsSameObject)
{
    YAML::Node node = YAML::Load("{key: value}");
    auto it_a = node.begin();
    auto it_b = node.begin();
    ASSERT_EQ(it_a, it_b);

    const auto& ref_a = *it_a;
    const auto& ref_b = *it_b;

    EXPECT_EQ(std::addressof(ref_a), std::addressof(ref_b)); // FAILS
}

// Control: std::forward_list::iterator is a true forward iterator.
// operator*() returns int& -- a stable reference to the stored node.
// https://en.cppreference.com/w/cpp/container/forward_list
TEST(ForwardListIterator, DereferenceOperator_WithEqualIteratorCopies_ReturnsSameObject)
{
    std::forward_list<int> list = {42};
    auto it_a = list.begin();
    auto it_b = list.begin();
    ASSERT_EQ(it_a, it_b);

    const auto& ref_a = *it_a;
    const auto& ref_b = *it_b;

    EXPECT_EQ(std::addressof(ref_a), std::addressof(ref_b)); // PASSES
}

Observed output from the yaml-cpp test:

Expected equality of these values:
  std::addressof(ref_a)  Which is: 000000D0A4B9EC50
  std::addressof(ref_b)  Which is: 000000D0A4B9ED50

The addresses differ -- each iterator copy holds its own iterator_value member, and operator*() returns it by value, materialising a new temporary per call.

Root cause

iterator_base<V>::operator*() (iterator.h) returns value_type by value:

value_type operator*() const
{
    // constructs and returns a new iterator_value every call
}

YAML::Node is a shallow-copying reference handle, not a value type. Each dereference constructs a new iterator_value{Node first; Node second;} wrapping the same underlying ref-counted data but at a new address. Two copies of the same iterator therefore dereference to two distinct C++ objects, violating [iterator.concept.forward]/6.

This is also related to #1326 (value_type being const-qualified), which is a separate but symptomatically similar violation of std::indirectly_readable.

Why this is a bug and not a design choice

Prior to C++20, iterator_category tags were advisory. In C++20, std::forward_iterator is a formal concept with normative semantic requirements. Code using std::views::transform or other range adapters on a YAML::Node will produce a forward_range view based on a false premise. Any algorithm that exploits multi-pass behaviour on that view has formally undefined behaviour, even if it happens to work in practice in a trivial single-pass use.

Proposed fixes

Option A (recommended): Change iterator_category to input_iterator_tag
The iterator is semantically an input iterator. Declaring it as such is honest and correct. Single-pass range-for loops and std::views::transform all work on input ranges. This is a breaking change for callers that pass the iterator to algorithms requiring ForwardIterator, but those callers already have latent UB.

Option B: Return a stable reference from operator*()
Store the current iterator_value inside the iterator as a member and return iterator_value&. However, since two equal iterators are independent copies each holding their own member, they still dereference to different objects. This does not fix the multi-pass violation and adds overhead.

Option C: Shared materialised state
All copies of the same logical iterator position share ownership of a single heap-allocated iterator_value. Satisfies the multi-pass guarantee mechanically, but adds allocation overhead and complicates copy semantics.

Option D (consuming-code workaround): Wrapper that re-declares input_iterator_tag
Users can wrap YAML::Node's iterator in a thin adapter that overrides iterator_category to input_iterator_tag. This is a local fix that does not require yaml-cpp changes, at the cost of boilerplate per use site.

Disclaimer: this bug was found by an AI coding tool (GitHub Copilot), with the assistance and review of a real human. This report was posted at the direct request of that human. The account posting this report is owned and operated by a real person.

Lingua principale
C++
Stelle
6.1k
Fork
2.2k
Merge medio
22h 2m
PR unite (30g)
1

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di jbeder/yaml-cpp

Tutte le issue di jbeder/yaml-cpp

Issue simili

Altre issue su C++

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.