Refactoring to environment

[krame505]

Refactor Scope/Contribs into something generally more useful

The implementation for an env namespace is also more generally useful for introducing new standalone environment attributes, whether flow-independant ones on the host language, or in creating new environments for embedded DSLs. I did some refactoring here, moving everything into Scope.sv and adding some more wrapper functions so that nobody else needs to worry about dealing with TreeMaps directly. I also renamed Scope to Scopes, since it is really a list/stack of individual scopes in a namespace.

Label environment

I did the refactoring discussed in #77. The actual implementation for this was fairly easy - there are now extra attributes labelDefs (replacing functiondefs) and labelEnv that are seperate from the main environment.
Figuring out the flow types has proven... difficult. There are a bunch of options that seem possible but actually don't work:

1. Have labelDefs depend on the reference set, and include labelEnv in the reference and forward sets. This is equivalent to what we are have now, which is an issue due to the cyclic dependency of labelDefs -> labelEnv -> forward -> labelDefs. To avoid infinite recursion, in practice we would need to avoid having labelDefs depend on the forward if the forward depends on anything using labelEnv. This is annoying but not too bad, just override the forward with an equivalent equation appending the child labelDefs and whatever labels you emit (note that this isn't interfering since the value that would have come from the forward is the same.) The biggest issue with this is that forgetting this override doesn't cause a flow error, just an infinite recursion crash at runtime. Accessing labelEnv from the label item reference would also cause a similar crash. In short, this approach is error prone and gets ugly quickly.
2. Same as 1, but exclude labelEnv from the forward set. Doesn't work because it is still part of the reference set, which is used by most attributes that would be useful in computing a forward, and thus we would be unable to forward based on things such as errors or typerep.
3. Same as 2, but also exclude it from the reference set. We now have the issue of how to calculate errors (which may require looking up a label) without exceeding the reference set.

This is the only workable option that I could think of:

4. Same as 1, but set the labelDefs flowtype to empty, treating it as a purely syntactic analysis similar to pp. This would also require an override of labelDefs on most forwarding productions (as discussed in option 1.) The other major drawback is that LabelItem may no longer contain a reference, so labelEnv only provides a list of all labels that exist, and not the decorated tree pointed to by the label. Not that we actually use this capability anywhere AFAIK... so maybe this isn't a big deal. Thoughts?

Add check that a declRefExpr refers to a value

Previously this wasn't being checked. I added an isItemValue attribute to ValueItem (mirroring isItemType, renamed from isItemTypedef), since we want errorValueItem to act both as a value and type.

Closed env item nonterminals

After some thought and experimentation, it really makes more sense to have the env item nonterminals (TagItem, ValueItem, MiscItem, etc.) closed, since we really care more about distinct types of environment items than about defining new analyses, as has been evidenced by, e.g. the algebraic-data-types extension, which is attempting to create a new kind of RefIdItem, but in an interfering way. Note that for this type of closed nonterminal, it does make sense to introduce new analyses checking if something is a specific kind of item, with a default value (e.g. a pattern match.) This is a case similar to the 'heterogeneous collection' structure that we introduced for Def in order to support extension env namespaces originally.

Another interesting side-effect of closing the ValueItem nonterminal is that this opens up directCallHandler as a more general-purpose extension point. Extensions can now introduce a new ValueItem production that has both isItemValue and isItemType left as false (raising an error if used directly in a declRefExpr or typedefTypeExpr), but with some extension production for directCallHandler. This introduces a way of dynamically creating 'built-in' functions, when it doesn't make sense to pass around a value with a new type that may be overloaded. This capability is somewhat analogous to C macros, and as such it could make sense to add a similar way of creating 'built-in' names with declRefExpr.
I have been using this technique in an extension that I have been developing as part of a class project, and it has proven quite useful - see here if interested.

TODO before this is merged

• Update extensions
• Check that tutorial on the environment is up to date

[krame505]

...And I just realized that if labelEnv needs to be part of the reference set, then there is absolutely no reason to have it seperate from the main env. Also, we can go back to having a general-purpose functionDefs attribute that does not depend on the reference or forward sets. I'll go ahead and change this back when I have time.

[krame505]

I made the above-mentioned change and updated the tutorials, in addition to some other random cleanup. Some of the extensions still are failing, but aside from that this is basically ready to merge.

[krame505]

I would like to get this merged rather soon, as there is a lot of important refactoring here. I also have some personal projects depending on this, and it is annoying to be doing everything from a seperate branch.

Currently, the only thing failing is the "parallel tree search" example in ableC_sample_projects. Basically, matchStmt forwards to warnStmt if there are any errors in its body, and makeFrame tries to evaluate cilkFrameDeclsScopes on matchStmt. This happens via forward, which attempts to error-check the body of the match, but this fails because it has the wrong env, not including the frame struct that we are currently building.

This cropped up due to some slight shuffling of env flow types, and it was really a fluke that this example even worked in the first place. I'm really not sure how to fix this issue, but it probably requires host language support to eliminate the forward depend cilkFrameDeclsScopes, or maybe a hack involving reflection. @TravisCarlson thoughts on this?

Anyway, if it is OK with @ericvanwyk I might just disable that example in Jenkins for now so this can merge.

[ericvanwyk]

Hi Lucas, The parallel tree search is an example that I used today with the colloquium speaker. So I would not want to disable these sorts of things. It is important that the demos work. Travis and I are working on a paper due on Sunday, so we may not have time to fix this issue this week. Can it wait a week?

…

-Eric

On Mon, Jan 29, 2018 at 7:27 PM, Lucas Kramer ***@***.***> wrote: I would like to get this merged rather soon, as there is a lot of important refactoring here. I also have some personal projects depending on this, and it is annoying to be doing everything from a seperate branch. Currently, the only thing failing is the "parallel tree search" example in ableC_sample_projects. Basically, matchStmt forwards to warnStmt if there are any errors in its body, and makeFrame tries to evaluate cilkFrameDeclsScopes on matchStmt. This happens via forward, which attempts to error-check the body of the match, but this fails because it has the wrong env, not including the frame struct that we are currently building. This cropped up due to some slight shuffling of env flow types, and it was really a fluke that this example even worked in the first place. I'm really not sure how to fix this issue, but it probably requires host language support to eliminate the forward depend cilkFrameDeclsScopes, or maybe a hack involving reflection. @TravisCarlson <https://github.com/traviscarlson> thoughts on this? Anyway, if it is OK with @ericvanwyk <https://github.com/ericvanwyk> I might just disable that example in Jenkins for now so this can merge. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#86 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ADOhd7brbCP-UpimsF7AaZZ6Nv0tJJM3ks5tPnAVgaJpZM4Qan0P> .

[krame505]

@ericvanwyk That is reasonable, but fixing this should still have a high priority since these examples are something that we use to test any changes to ableC. I'll open a bug report in ableC-cilk.

[ericvanwyk]

Thanks for opening the bug report - @TravisCarlson and I can get to this next week.

[krame505]

I finally got this to pass Jenkins, so I'd like to merge soon. @ericvanwyk or @tedinski could one of you submit a review for this when you get a chance?

[tedinski]

Not that we actually use this capability anywhere AFAIK... so maybe this isn't a big deal. Thoughts?

Make sure there's a comment in there explaining why it's done this way.

Other than that, if everything is green, go for it. I did see some things in jenkins that were red ('develop' branches built against your branch of ablec), maybe this was just a side effect of some jenkins things being done poorly at the moment, but do me a favor and:

1. I'm working on Jenkins stuff until 6pm today. Please don't merge until after that/tomorrow/later/etc.
2. Please merge when you have time to babysit the build and fix anything if it fails. I'd like everything to stay building successfully. And it does take like an hour at the moment... (I'm trying to cut that down now.)

[krame505]

Make sure there's a comment in there explaining why it's done this way.

No problem.

I did see some things in jenkins that were red ('develop' branches built against your branch of ablec), maybe this was just a side effect of some jenkins things being done poorly at the moment

Yeah, there are some corresponding changes needed in extensions as a result, on their respective feature/env branches. I think some of the Jenkinsfiles are still old and try building against develop when feature/env failed in the past.
...speaking of which, what is your recommended procedure for this sort of simultaneous merging? If I merge the extensions first then they will have some failed builds initially, but if I merge ableC first then I need to quick merge all the extensions before ableC finishes triggers them. What would be nice is having some way of disabling builds for just the extensions, until ableC has been merged.