Fails to parse other map keys after a sequence if the sequence items are not indented

[leoetlino]

First, let me thank you for making this library! I am currently in the process of switching from PyYAML to ryml for performance reasons, and so far the initial results are very encouraging. ryml is typically 20-25x faster for emitting and one file that took 2s to emit with PyYAML or yaml-cpp only takes ~90ms with ryml.

However, parsing existing documents that were emitted by PyYAML (or libyaml) is proving to be a bit more problematic.

Currently any documents that look like this fail to parse with ryml:

enemy: #L1
- actors: #L2
  - {name: Enemy_Bokoblin_Junior, value: 4.0} #L3
  - {name: Enemy_Bokoblin_Middle, value: 16.0} #L4
  - {name: Enemy_Bokoblin_Senior, value: 32.0} #L5
  - {name: Enemy_Bokoblin_Dark, value: 48.0} #L6
  species: BokoblinSeries #L7
# ...

/home/leo/projects/oead/src/lib/rapidyaml/src/c4/yml/parse.cpp:597: ERROR parsing yml: parse error
line 7: '  species: BokoblinSeries' (sz=25)
           ^~~~~~~~~~~~~~~~~~~~~~~  (cols 3-26)
top state: RSEQ|RNXT

If the actors array is indented as follows:

enemy:
- actors:
    - {name: Enemy_Bokoblin_Junior, value: 4.0}
    - {name: Enemy_Bokoblin_Middle, value: 16.0}
    - {name: Enemy_Bokoblin_Senior, value: 32.0}
    - {name: Enemy_Bokoblin_Dark, value: 48.0}
  species: BokoblinSeries

then the file is parsed correctly. It seems that the lack of extra indentation for the array is throwing ryml off and causing it to miss the end of the sequence?

[cburgard]

Possibly not even related to map keys. I get the same message here:

meas:
  createParameterList:
    - Lumi
      value: 1
      relErr: 0.1
    - mu
    - alpha_syst1
    - mu
      value: 1
      low: 0
      high: 3
      const: 0

ERROR parsing yml: parse error - indentation should not increase at this point
line 4: '      value: 1' (sz=14)
         ^~~~~~~~~~~~~~  (cols 1-15)

[leoetlino]

Hmm, well I'm not sure that document is well formed? Is each sequence/array item supposed to be a map? If so, you'd need to make "Lumi" and the last "mu" either keys or values. For what it's worth, PyYAML doesn't like it either.

[cburgard]

Well, the document was written with rapidyaml. So irrespective of whether it's valid YAML or not, I'd hope it would at least be able to parse back what it emitted :-)

[leoetlino]

Heh, indeed, though that really does sound more like a bug with the emitter as the YAML is invalid.

[biojppm]

@leoetlino Thanks for writing up.

Your YAML is valid indeed, so this is a parser bug. The parser code in ryml has trouble with the first non-indented sequence item on line 2. From there on, all bets are off. So you are right that indenting the sequence will allow ryml to parse it correctly.

[biojppm]

@cburgard - I've filed a separate bug #31 to track that problem; thanks for writing up.

[leoetlino]

So I did a small test, and it looks like changing Parser::_handle_seq_impl (in the RNXT state) to fall back to assuming the sequence was unindented and ending it lets ryml parse the example document correctly:

rapidyaml/src/c4/yml/parse.cpp

Lines 589 to 598 in e1216d4

	else if(rem.begins_with("---"))
	{
	_c4dbgp("got document start '---'");
	_start_new_doc(rem);
	return true;
	}
	else
	{
	_c4err("parse error");
	}

-            _c4err("parse error");
+            _c4dbgp("end the indentless sequence");
+            _pop_level();
+            return true;

enemy:
- actors: 1
  test_array:
    - a
  test: x
test: 2

parses correctly into this:

enemy:
  -
    actors: 1
    test_array:
      - a
    test: x
test: 2

That is most likely just a hacky workaround though. I'm not sure how to fix this correctly.

[biojppm]

@leoetlino thanks for the note - it did help. All the unit tests pass, so I'm going to go with that.

This is in the dev branch, and I'm going to look next at #32 in the same branch. When it's fixed, I'll merge dev to master, and only then will I close.