Consistently set & propagate child names

[asl]

child_name is only updated on the context if it is non-empty. Looks like it was a ad-hoc approach to propagate child_name for Vector / IndexedVector as these do not have context and visitation of them looks like as we just unroll the vector.

However, these caused unwanted side effects in many places (and could be easily seen via e.g. dumper pass). Consider, for example, P4Parser:

void IR::P4Parser::visit_children(Visitor & v) const {
    Type_Declaration::visit_children(v);
    v.visit(type, "type");
    v.visit(constructorParams, "constructorParams");
    parserLocals.visit_children(v);
    states.visit_children(v);
}

Here the child_name inside parserLocals.visit_children(v) and states.visit_children(v) would be constructorParams and this is not something we'd want (note multiple constructorParams fields):

  [671] P4Parser name=p declid=1
    type: [30] Type_Parser name=p declid=0
      typeParameters: [5] TypeParameters
      applyParams: [26] ParameterList
        [10] Parameter name=check declid=0 direction=in
          type: [8] Type_Boolean
        [18] Parameter name=foo declid=1 direction=in
          type: [16] Type_Bits size=10 isSigned=0
        [23] Parameter name=matches declid=2 direction=out
          type: [21] Type_Boolean
    constructorParams: [153] ParameterList
    constructorParams: [490] Declaration_Variable name=val1 declid=12
      type: [2] Type_Bits size=32 isSigned=0
      initializer: [487] Cast
        type: [2] Type_Bits size=32 isSigned=0...
        expr: [486] Constant value=2 base=10
          type: [2] Type_Bits size=32 isSigned=0...
        destType: [2] Type_Bits size=32 isSigned=0...
    constructorParams: [696] ParserState name=start declid=4

Here we extend visit_children to accept a name argument and use to to pass the upper-level field name. Example above becomes:

  objects: [671] P4Parser name=p declid=1
    type: [30] Type_Parser name=p declid=0
      typeParameters: [5] TypeParameters
      applyParams: [26] ParameterList
        parameters: [10] Parameter name=check declid=0 direction=in
          type: [8] Type_Boolean
        parameters: [18] Parameter name=foo declid=1 direction=in
          type: [16] Type_Bits size=10 isSigned=0
        parameters: [23] Parameter name=matches declid=2 direction=out
          type: [21] Type_Boolean
    constructorParams: [153] ParameterList
    parserLocals: [490] Declaration_Variable name=val1 declid=12
      type: [2] Type_Bits size=32 isSigned=0
      initializer: [487] Cast
        type: [2] Type_Bits size=32 isSigned=0...
        expr: [486] Constant value=2 base=10
          type: [2] Type_Bits size=32 isSigned=0...
        destType: [2] Type_Bits size=32 isSigned=0...
    states: [696] ParserState name=start declid=4
      selectExpression: [699] SelectExpression
        type: [548] Type_State
        select: [701] ListExpression
          type: [516] Type_List
            components: [446] Type_Bits size=10 isSigned=0
            components: [510] Type_List
              components: [2] Type_Bits size=32 isSigned=0...
              components: [2] Type_Bits size=32 isSigned=0...

[ChrisDodd]

So the child_name field was originally added to the context to aid in debugging -- when in gdb, you can look at the context and see this string to get a better idea where it was. Then it got used in the dump code to dump IR and that worked fine too. The main issue is that when dumping the children of an IR::Vector, they did not have names just indexes, so in Vector::visit_children, this field would not be set. This caused the dumper to output the elements of a Vector in sequence without a name: prefix.

This all worked fin until we introduced inline Vectors -- in this case the children were children of the Vectors parent ans so things got messed up.

I always wanted to improve the dumper to print the indexes for Vectors, but never came up with a good way of doing it without being too expensive -- we don't want to add overhead to all IR traversals (by, for example, creating dynamic strings to put in child_name) as this is something that is there to help debugging (the overhead of just storing an extra pointer to a static string for each node visited is not too bad).

Overall this looks good and is definitely an improvement. I'd still like to figure out a way to print indexes so you would end up with something like:

    type: [30] Type_Parser name=p declid=0
      typeParameters: [5] TypeParameters
      applyParams: [26] ParameterList
        parameters[0]: [10] Parameter name=check declid=0 direction=in
          type: [8] Type_Boolean
        parameters[1]: [18] Parameter name=foo declid=1 direction=in
          type: [16] Type_Bits size=10 isSigned=0
        parameters[2]: [23] Parameter name=matches declid=2 direction=out
          type: [21] Type_Boolean

without introducing extra overhead. My thought was to add some extra magic char to the static string that IRDumper could recognize and do something with, but I never got that far.

[asl]

Overall this looks good and is definitely an improvement. I'd still like to figure out a way to print indexes so you would end up with something like:

Yeah, I wanted something like this as well, but thought that at least name is good enough :) Otherwise things were pretty misleading. Also note that the code in DoSynthesizeActions::preorder(IR::AssignmentStatement*) explicitly relies on child_name and wants it set (and therefore implicitly relies on stale name in the context :) )