yaml-cpp
yaml-cpp copied to clipboard
jbeder
Node gets modified after query
This code asserts for me in the last IsSequence:
YAML::Node node = YAML::Load("r:\n- a: [1,2]");
auto r = node["r"];
for (auto& c : r)
{
auto a = c["a"];
assert(a.size() == 2);
assert(a.IsSequence());
auto b = a["b"];
assert(a.size() == 2);
assert(a.IsSequence());
}
I think i understand
Querying for keys does not create them automatically (this makes handling optional map entries very easy)
wrong, because it seems to be ensured that queried nodes exist after querying. How can i safely and easily query for existence of map entries without creating them?
The const version doesn't have this property:
YAML::Node const node = YAML::Load("r:\n- a: [1,2]");
auto const r = node["r"];
for (auto const& c : r)
{
auto const a = c["a"];
assert(a.size() == 2);
assert(a.IsSequence());
auto const b = a["b"];
assert(a.size() == 2);
assert(a.IsSequence());
}
@jessesna Can you please provide the sample yaml that you were trying to read, I will try to reproduce the issue.
@jbeder Can I work on this issue? Can you please assign the issue to me?
Can you please provide the sample yaml that you were trying to read, I will try to reproduce the issue.
Great! The yaml in the example from the initial post should reproduce it:
r:\n- a: [1,2]
Hi @jbeder, I am thinking of following possibilities to do this:
-
I will introduce a new bool member variable for class Node, say m_InsertOnNotFound, based on this the functionality of as Node object will vary. The truth value of the member variable defaults to false, dont create entries if not found. The value of this can be changed via Load->NodeBuilder->Node. This will be needing a lot of changes across source code files.
-
Or maintain a global variable g_InsertOnNotFound(default false, don't create entries if not found), which can be changed via the Load function calls for that particular document, and based on this variable we can alter the get functions in node implementations to create a new node or just quit/throw an error on not found?
Once a Load function is called on a YAML doc/string, this behaviour remains same for that document/string, so I am thinking the global variable approach would be nice to implement.
What are your thoughts on this? Any suggestions/comments?
Thanks.
Why not just add an alternative method to query Node, like Node::at() const or Node::value() const? I would be really unhappy if I were to deal with some mode switches (and the mention of a global variable just scared the hell out of me).
Yes actually that sounds a lot better than my approach. I didn't think of that way.
I'll follow that idea. Thank you Alexey Rusakov for correcting me.
On Sat, Dec 11, 2021, 17:41 Alexey Rusakov @.***> wrote:
Why not just add an alternative method to query Node, like Node::at() const or Node::value() const? I would be really unhappy if I were to deal with some mode switches (and the mention of a global variable just scared the hell out of me).
— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/jbeder/yaml-cpp/issues/1068#issuecomment-991620457, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACTWC75VG2OY3X6XRHD3KQLUQM5V5ANCNFSM5IUXWFOQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
Hi everyone, for the past 15 days, I was on vacation with no access to internet, so couldn't work on this. Now I am back and started working on this issue, will close this soon.
@jessesna After going through the code internals, I have tried your example, the const version, it is working fine for me, below are the code and output that I am getting on my Ubuntu 20.04.3 LTS machine using a clang++ compiler. Code:
#include <iostream>
#include <assert.h>
#include "yaml-cpp/yaml.h"
int main() {
YAML::Node const node = YAML::Load("r:\n- a: [1,2]");
auto const r = node["r"];
for (auto const& c : r)
{
std::cout << "Inside for loop at line number " << __LINE__ << "\n";
auto const a = c["a"];
assert(a.size() == 2);
assert(a.IsSequence());
auto const b = a["b"];
if (!b) { std::cout << "Node b is a ZombieNode created here.\n"; }
assert(a.size() == 2);
assert(a.IsSequence());
}
// emit the YAML
YAML::Emitter out;
out << node;
std::cout << "Following is the YAML:\n-------------------\n";
std::cout << out.c_str() << "\n----------------\n";
}
Output:
Inside for loop at line number 10
Node b is a ZombieNode created here.
Following is the YAML:
-------------------
r:
- a: [1, 2]
----------------
And the non-const version example is as follows: Code:
#include <iostream>
#include <assert.h>
#include "yaml-cpp/yaml.h"
int main() {
YAML::Node node = YAML::Load("r:\n- a: [1,2]");
YAML::Emitter out;
out << node;
auto r = node["r"];
for (auto c : r)
{
std::cout << "Inside for loop at line number " << __LINE__ << "\n";
auto a = c["a"];
assert(a.size() == 2);
assert(a.IsSequence());
a["b"] = "NewNode";
std::cout << "Modified the Node.\n";
}
// emit the YAML
out << node;
std::cout << "Following is the YAML:\n-------------------\n";
std::cout << out.c_str() << "\n----------------\n";
}
Output:
Inside for loop at line number 12
Modified the Node.
Following is the YAML:
-------------------
r:
- a: [1, 2]
---
r:
- a: {0: 1, 1: 2, b: NewNode}
----------------
And as far as I understood this is what you want, you can use const version without changing the node structure and if you want to change the node structure, you can use non-const version. Let me know if I am wrong somewhere or you need more clear explanation.
Thanks Rajkumar Ananthu
Hi @rajkumarananthu
And as far as I understood this is what you want, you can use const version without changing the node structure and if you want to change the node structure, you can use non-const version.
It's not what i'd want or expect.
This code asserts for me in the last IsSequence:
YAML::Node node = YAML::Load("r:\n- a: [1,2]");
auto r = node["r"];
for (auto& c : r)
{
auto a = c["a"];
assert(a.size() == 2);
assert(a.IsSequence());
auto b = a["b"];
assert(a.size() == 2);
assert(a.IsSequence());
}
From my understanding of the documentation
Querying for keys does not create them automatically (this makes handling optional map entries very easy)
and from a intuitive understanding of such an API, i don't see any reason why a query should alter the underlying structure. What advantage does the current implementation offer? What is the rationale behind changing the sequence to a map without actually assigning a value to the new key?
Hi everyone thank you for the waiting.
@jessesna I found that the non-const version of operator[] is implemented like that is to facilitate the node creation/manipulation in the following way:
Node node;
node["a"] = "b";
For this reason, it is not possible to change the existing method, the workaround for this is to use the const version only, otherwise I can provide another function as @KitsuneRal suggested so that you can access the node without changing the node structure.
If you have any other suggestions please let me know, I am open for learning.
@jbeder you have any comments to add here.
Thanks, Rajkumar Ananthu.
I just found that it's a little unintuitive and doesn't fully match the documentation.
What i was expecting was some sort of delay so that even a writable instance would only add to the instance as soon as it's used to assign a value, e.g.
YAML::Node node = YAML::Load("r:\n- a: [1,2]");
auto r = node["r"];
for (auto& c : r)
{
auto a = c["a"];
auto b = a["b"]; // no observable change in a yet, just returning a proxy or something
b = "a"; // observable change in a delayed until here
}
This might be a philosophical debate but for a bit similar reasons i avoid the index operator of std::map and always use find or insert.