More precise use-defs in rename code#1113
Conversation
…ge of `tm.useDef` is a subset of `tm.definitions`
sungshik
changed the title
toinehartman
left a comment
toinehartman
left a comment
There was a problem hiding this comment.
Good stuff! I cannot completely understand how these changes will influence uses of the use-def relation that are not used to index tm.definitions.
I would expect the main benefit of this TModel extension to be that we can override (one of) the following functions from the rename framework. As you mentioned off-line, this is something we can address in the future.
I think with the addition of tm.define2id, we should be able to implement the following without any tree visits.
Or even the (only) function that calls the above:
| visit (tr) { | ||
| case (Expression) `<Expression e>(<{Expression ","}* _> <KeywordArguments[Expression] kwArgs>)`: { | ||
| funcKwDefs = keywordFormalDefs[tm.useDef[e.src]]; | ||
| funcKwDefs = keywordFormalDefs[getUseDef(tm)[e.src]]; |
There was a problem hiding this comment.
It seems expensive to do this inside the visit, but since we're modifying the use-defs inside this visit as well, it might be necessary to do it here...
There was a problem hiding this comment.
Good point! The use-defs are modified during the visit, but as far as I understand the code: (a) entries are only created, not deleted, not updated; (b) none of the created entries have e.src as use. So, I think it is safe to call getUseDef only once, before the visit, because the subset of it that is read during the visit doesn't change.
I made the change, ran the tests, and everything seems fine. What do you think?
|
2 participants
This PR updates the rename code to make it compatible with companion PR usethesource/rascal#2801, which updates the computation of use-def relations.