certik / yaml-cpp

Automatically exported from code.google.com/p/yaml-cpp
MIT License
0 stars 0 forks source link

Using YAML::Node as key doesn't do deep equality checking #273

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
cout << "'" << YAML::Load("foo: bar")[YAML::Load("foo")] << "'"
  << "== '" << YAML::Load("foo: bar")["foo"]             << "'" << endl;

What is the expected output? What do you see instead?
Expected 'bar' == 'bar', see '' == 'bar'.

What version of the product are you using? On what operating system?
  ubuntu libyaml-cpp-dev 0.5.1

Please provide any additional information below.
  YAML::Node::operator[](const YAML::Node&) does only shallow equality checking on the internal node pointer instead of deep equality checking of the node contents, thus breaking down if the Node's come from different graphs (or from different parts of the same graph) albeit with same content. 
While I'd consider this a defect, I accept the argument that the node equality 
issue is not trivial and that the current API works as intended, but is just 
merely missing a deep-comparing indexing mechanism (or at least I wasn't able 
to find one). In this case, this report can be considered an enchancement 
request for adding one.

Original issue reported on code.google.com by iridian....@googlemail.com on 2 Feb 2015 at 10:15

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
The full use case where I encountered this issue is when doing recursive 
merging of a target YAML graph by an updated, overlapping sub-graph as follows:

void mergeToNode(YAML::Node source, YAML::Node target) {
 if (source.Type() == YAML::NodeType::Map) {
   for (YAML::Node::const_iterator i = source.begin(); i != source.end(); ++i)
     mergeToNode(i->second, target[i->first]);
   else // Missing support for sequence merging here etc.
     target = source;
}

Here the problem is that target[i->first] doesn't do deep equality comparison 
but the shallow internal node-pointer based one. Because of this, it never 
finds the old branches of the target graph and ends up adding the data as 
duplicate data instead of replacing the old data.

I'm new to YAML, if there's a way to do this kind of partial graph update 
somehow in a nicer fashion, I'll be happy. I see there was some discussion on 
YAML features (the '<<' notation) but that was missing from yaml-cpp apparently?

Original comment by iridian....@googlemail.com on 2 Feb 2015 at 10:25

GoogleCodeExporter commented 9 years ago
This is basically working-as-intended. yaml-cpp does identity comparisons so 
you can build recursive data structures. However, you can lookup by a typed-key 
with your own equality, and that will lookup via equality. E.g.

struct Foo { ... };
bool operator ==(const Foo& a, const Foo& b) { ... }
// define appropriate conversion operators as in the docs

Foo x = ....;
node[x] = ...;

This will look up any key that's equal to x according to the user-defined 
equality.

Original comment by jbe...@gmail.com on 3 Feb 2015 at 6:32

GoogleCodeExporter commented 9 years ago
Is there an function that could be used to do deep-equality testing?

I feel like this functionality should be part of the library, because it's both 
a generic concept and also requiring a user to implement that manually is 
probably much more work than if it's implemented on the library side. Then 
again, if there is not even scalar YAML::Node level equality operator, I guess 
a deep-equality can't really be offered, which is a bummer.

For my actual use case, I just did some explicit checking for the "i->first" 
type and things like target[i->first.as<string>()] to get around, but it's not 
a generic solution.

Original comment by iridian....@googlemail.com on 3 Feb 2015 at 6:52

GoogleCodeExporter commented 9 years ago
I see you opened Issue 274 about this. Feel free to continue discussion there - 
but this is unlikely.

Original comment by jbe...@gmail.com on 3 Feb 2015 at 2:09