Issues with Merge Keys

[sifferman]

Hello! I have been using FuseSoC and Edalize for a few years now, and I've run into a lot of issues with the merge key feature from YAML 1.1. I wanted to open a discussion about examples, workarounds, and possible permanent fixes.

_append strange behavior

If a child and grandchild both have a field with _append, then the grandchild overwrites the child's field. I would expect that both _appends are added to the parent's field.

This is already described in #624

Example

targets:
  default: &default
    filesets: [rtl1]

  child: &child
    <<: *default
    description: inherits from default
    filesets_append: [rtl2]

  grandchild:
    <<: *child
    description: inherits from child
    parameters_append: [rtl3] # overwrites rtl2

Workaround

targets:
  default: &default
    filesets: [rtl1]

  child: &child
    <<: *default
    description: inherits from default
    filesets_append: [rtl2]

  grandchild:
    <<: *child
    description: inherits from child
    parameters_append: [rtl2, rtl3] # must insert both

Subfields are not copied

If a key exists in a child that matches a key from the parent, none of the subfields from the parent are given to the child. I would expect that the parent fields would be treated as defaults, then the child overwrites what it wants.

Example

targets:
  default: &default
    tools:
      verilator:
        mode: cc
        verilator_options:
          - --timing
  child:
    <<: *default
    tools:
      verilator:
        mode: lint-only
        # does not inherit verilator_options

Workaround

targets:
  default: &default
    tools:
      verilator:  &verilator
        mode: cc
        verilator_options:
          - --timing
  child:
    <<: *default
    tools:
      verilator:
        <<: *verilator # grab the subfield directly
        mode: lint-only

Possible Solution

From these examples, it becomes clear that the merge key operation has some flaws.

One solution may be to create a new FuseSoC-only operator, (maybe $<), that fixes the above issues. I would be happy to add this functionality if people feel it would be valuable. I imagine that the best place to handle this is in CoreParser.read():

https://github.com/olofk/fusesoc/blob/40ca3f4765897c969c727dac1adba865d2c7148d/fusesoc/parser/coreparser.py#L49-L55

Thoughts?

[olofk]

The _append functionality is very simplistic, and in hindsight I wonder if it was really a good idea in the first place. I agree that the current behavior is unexpected though and I'm open to proposals here. I do want to make sure they don't end up overly complex though, because at some point it might be easier to just require the user to add all filesets manually or treat the common code as a dependency instead.

Regarding the subfields I agree that this behavior should be changed to how you describe it. IIRC that was the intention