-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fixes in tuple handling + output format improvements #78
Conversation
analysis/dataflow/mark.go
Outdated
type IndexKind int | ||
|
||
const ( | ||
// ReturnedTupleIndex is index on a tuple returned by a functoin |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: typos
for outPath := range outPaths { | ||
if inPath != "" || outPath != "" { | ||
if !prefixed { | ||
prefix += "@" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't need to be addressed in this PR but I'm wondering if we should come up with a more efficient representation for paths? Strings are relatively expensive to update because they are immutable. Maybe it would be more efficient to use [][]byte
or []string
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could, the limitation is that we need to match the pointer analysis' representation. Currently there is a 1-1 mapping between the two, they are both strings and have the same "syntax". Now that we have the pointer analysis internally, we can change that representation. We could also add some parsing step to translate between representations.
analysis/dataflow/mark.go
Outdated
// ReturnedTupleIndex is index on a tuple returned by a functoin | ||
ReturnedTupleIndex IndexKind = iota | ||
// NonIndex is a non-index | ||
NonIndex |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't NonIndex
be the default value (0)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I use the default value, but that's a good point, it should be.
@@ -76,6 +87,22 @@ func (m MarkType) String() string { | |||
} | |||
} | |||
|
|||
// MarkIndex wraps an index kind and the index value together | |||
type MarkIndex struct { | |||
// Kind is the kind of index (currently, either a real index or not) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we plan to have other kinds of indices?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking was that some point we might want to track indices resulting from next
and select
instructions as a special case.
This PR improves tuple handling in the dataflow analysis: