Inconsistency bug or a feature in 0.7.0

[leonid-butenko]

THe following code test yaml and the accompanying code snippet fails on the second try:

the test yaml:

Test:
  Level1: "Some string"

The code snippet

    YAML::Node cfg = YAML::LoadFile(absname);

    YAML::Node currentNode = cfg;
    currentNode = currentNode["Test"];
    std::cout << "first: " << currentNode["Level1"].as<std::string>() << std::endl;

    currentNode = cfg;
    currentNode = currentNode["Test"];
    std::cout << "second: " << currentNode["Level1"].as<std::string>() << std::endl;

output:

first: Some string
bad conversion

if I now comment out the second occurance of "currentNode = currentNode["Test"];" assignment, the output is as expected. Bug or feature?

I use version 0.7.0 from conda-forge.

Any ideas?

[KitsuneRal]

I just stumbled over a similar thing, as of 0.8.0. If you put assert(currentNode != cfg); after the first currentNode = currentNode["test"]; you'll be surprised to see the assertion failing because instead of detaching currentNode the assignment updated the common storage underneath currentNode and cfg. And that only seems to happen once; if you do another assignment (not from cfg) it will get detached, it seems. Looks like a bug.

[KitsuneRal]

Actually, this seems to be ultimately the same issue as #391; #516 is also related.

[KitsuneRal]

So, basically, with some arbitrary YAML map like:

key1:
  key2:
    key3: value

The following code will act in a rather unexpected way:

    const auto yamlDoc = YAML::LoadFile(/* that file above */);
    YAML::Node y = yamlDoc;
    y = y.begin()->second;
    assert(y == yamlDoc); // yamlDoc gets dragged down the hierarchy along with y, as both refer to the same memory
    y = y.begin()->second;
    assert(y != yamlDoc); // But that only happens once; at this point yamlDoc is left behind at key2, while y is at the map with key3

[Arech]

Yes, this broken const-correctness & copy-correctness breaks each and every attempt to compose yaml parsing and is extremely annoying. Essentially, a Node object isn't a node, but just some heavy handle to an internal node representation which can change its state even in const function invocation. So you don't copy nodes when you assign them, you copy handles that additionally change their state during that... Unbelievable. Reported years ago and still not fixed and not even mentioned in the docs.

[jbeder]

Yes, this is a design trade off. Probably should be in the docs, agreed. PRs welcome.

"Fixing" it would require a major API change so I'd want to be really careful about it. If you have a good idea, I'm open to it also.

[Arech]

@jbeder this causally undocumented "design trade off" already cost me days of frustrated debugging... I can't even tell what I feel about that.

This thing is critically important to be at least documented so others wouldn't waste time on it. Please do it.

I have some ideas how to mitigate that, but for that I need to understand how do I create/return a Node object that will test static_cast<bool>(obj) == false.

Essentially the idea is to YAML::Clone() every top-level "node" that needs to be accessed later, before reassigning its object to some other node. But to make it work, since node existence is generally tested by casting to bool, I need a way to create/return an empty object that would test negative and signify "node doesn't exist". Default constructed Node objects have static_cast<bool>(obj) == true, since they have m_isValid==true and no m_pNode. .reset() member function doesn't help either. How do I do that?

[jbeder]

I'm sorry this cost you lots of time. Unfortunately, I don't have time to work on this library any more. But patches welcome!