-
Notifications
You must be signed in to change notification settings - Fork 8
Refactor flow analysis to track deps on syn and eq subtree vertices #913
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
krame505
wants to merge
74
commits into
develop
Choose a base branch
from
feature/stitch-syns
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…rtices in stitch points for constructed trees
…at is always a rhsVertexType
…lar ones to avoid re-computation of stitch edges
…a nt stitch points, yikes
… revert misguided changes to sig sharing stitch points and dec site resolution
… edges from the tile graph
…owup in flow graph sizes This is somewhat less precise (and thus more conservative) than using tile stitch points for the entire tree, but in most cases shouldn't matter.
… decoration site tree resolution. Not sure why this check was added originally.
… by the attr occurence, new check for extension dispatch impl override eqs, update tests
…n site vertex types, since they are decorated trees
…s prods have singletons
…t for suppressed copy eq due to trans attr override on a forwarding prod
…ices for the equation deps of the outer tree node needed when accessing an attribute, and the full equation deps when taking a reference. TODO: lhsEqVertex seems like the wrong way to deal with references to the LHS
…s on lhs.eq in tile stitch points
…nger depend on the reference eqs
Eliminate checkAllEqDeps and move all inh eq checks to reference/attr access expressions. Also refactor handling of pattern match on a reference. Currently, still has some bugs related to pattern variables, that also (ab)use anon vertex types.
…translation attributes on lhs
…f action code, which may depend on errors
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #907. Fix #909.
Changes
Massive flow analysis refactor. This fixes a hole in the flow analysis for tree sharing, in which dependencies in constructed productions on their children and children's syn attributes were not being considered in the sharing production. This yields more specific error messages than the "hidden transitive dependency" one in many cases, since we can now point to where a flow type is actually being exceeded, rather than "there may be a flow type exceeded somewhere". This change also somewhat improves the precision of the analysis for specifications that do not use sharing, as we can now more precisely determine the dependencies of a synthesized attribute on a tree, when we know by what production that tree was constructed.
Major changes
Performance optimizations
checkAllEqDepsis no longer required and can be eliminated, substantially reducing the number of places we check for the presence of inherited equations (and query transitive dependencies.)checkAllEqDeps, like action block code.Ord/EqonFlowVertexto compare avertexNameattribute, which is cached on theFlowVertex. String comparison is much faster than traversing a (now potentially deeply) nestedVertexTypetree.FlowVertex(using a newIdCachelibrary.) This allows for vertex comparisons to be done using a single integer comparison, instead of the potentially-longvertexName.Note that there is not a huge amount of change in the performance of the analysis; the above-mentioned optimizations more or less offset the overhead of the increase the in size (and number) of flow graphs.
Other changes
Documentation
Updated comments, mostly. There actually aren't very many user-visible changes here, mostly just better (and more correct) error messages.
I'll try to update the website at some point to discuss the
--warn-sharing-cyclesflag.Testing
Tested by finding new flow errors in the Silver compiler and downstream projects. Also added and updated a number of new tests in the testsuite.