mergeQualifiers should be parameterized by a corresponding inherited attribute

[krame505]

Currently mergeQualifiers<a> has type (a ::= a). Instead should have type a, and there should be a corresponding inherited attribute mergeQualifiersWith<a>::a (or whatever you want to call it) that is used as the 'parameter' to this analysis instead. This is mainly a stylistic thing for consistency, since right now it doesn't really jive with how we do this sort of transformation in attribute grammars, but also in order to make things considerably less weird for extensions, if they want to override this equation on a forwarding Type (in a non-interfering way, of course - one may simply want to 'do the best job possible' in preserving their custom type, in order to maintain pp, etc.)

[tedinski]

This is mainly a stylistic thing for consistency, since right now it doesn't really jive with how we do this sort of transformation in attribute grammars

I wouldn't put too much weight on this one. We haven't really explored doing things this way much yet, imo. We didn't have lambdas and rich enough function types before, and nowadays we do. Function attributes could turn out to be a great tool.

Also, it's not only style, there is one big semantic difference: With a Decorated Nt you can call the function attribute with different parameters or parameters that would be hard to thread to the decoration site. With an inherited attribute, you probably have to consider some awkward "undecorate and redecorate" dance.

but also in order to make things considerably less weird for extensions, if they want to...

But of course if there's actually a good reason, go for it. Mind showing me a tiny example of what this weirdness looks like? It'd be interesting to accumulate some more experience about when a function attribute might be a bad idea...

[krame505]

I ran into this for the closure extension, wanting to override the mergeQualifiers attribute on closureType, to preserve closureType for the sake of pp when possible. In order to do this, I was thinking you would need to pattern match on the parameter to mergeQualifiers in order to determine whether to use forward.mergeQualifiers or a different function... although now that I think about it, you could just do

  top.mergeQualifers = \ otherType::Type ->
     case otherType of
      closureType(...) -> ...
    | _ -> forward.mergeQualifiers(otherType)
    end;

which I guess isn't as bad as I thought.

Also, it's not only style, there is one big semantic difference: With a Decorated Nt you can call the function attribute with different parameters or parameters that would be hard to thread to the decoration site. With an inherited attribute, you probably have to consider some awkward "undecorate and redecorate" dance.

Yeah, good point. But this is a Type, which we never pass as a reference anywhere.

[TravisCarlson]

I started working on this by splitting the attribute into synthesized withMergedQualifiers and inherited mergeQualifiersWith. Equations for these attributes then looked like:

abstract production pointerType
top::Type ::= q::Qualifiers  target::Type
{
  top.withMergedQualifiers =
    case top.mergeQualifiersWith of
      pointerType(q2, _) ->
        pointerType(unionQualifiers(top.qualifiers, q2.qualifiers),
                    target.withMergedQualifiers)
    | _ -> pointerType(q, target)
    end;
  target.mergeQualifiersWith =
    case top.mergeQualifiersWith of
      pointerType(_, target2) -> target2
    | _ -> error("?")
    end;
}

I didn't like the style of duplicating these case expressions, so my first thought was to create a function mergeQualifiers that's used like this:

abstract production pointerType
top::Type ::= q::Qualifiers  target::Type
{
  top.withMergedQualifiers =
    case top.mergeQualifiersWith of
      pointerType(q2, target2) ->
        pointerType(unionQualifiers(top.qualifiers, q2.qualifiers),
                    mergeQualifiers(target, target2))
    | _ -> pointerType(q, target)
    end;
}

function mergeQualifiers
Type ::= t1::Type  t2::Type
{
  t1.mergeQualifiersWith = t2;
  return t1.withMergedQualifiers;
}

But then I realized that this isn't much different than the original solution and probably doesn't address your issue.

What would your ideal mergeQualifiers equation on closureType look like?

[krame505]

That is pretty much exactly the refactor I was proposing. If we do this refactoring, we should for sure have a seperate mergeQualifiers function just like what you have. Stylistically I prefer the first option, but in some cases (like with closureType) doing so is hard, if we have lists of Types. In this case, we would probably have a combination of both approaches, something like

abstract production closureType
top::Type ::= q::Qualifiers params::[Type] res::Type
{
  ...
  top.withMergedQualifiers =
    case top.mergeQualifiersWith of
      closureType(q1, params1, _) ->
        closureType(
          unionQualifiers(q1.qualifiers, q2.qualifiers),
          zipWith(mergeQualifiers, params, params1),
          res.withMergedQualifiers))
    | _ -> forward.withMergedQualifiers
    end;
  res.mergeQualifiersWith =
    case top.mergeQualifiersWith of
      closureType(_, _, res1) -> res1
    end;
  ...    
}

But after thinking about Ted's comments, I could also be convinced that leaving it the way it is is just fine. However either way I think we for sure should still add a seperate mergeQualifiers :: (Type ::= Type Type) function.