-
Notifications
You must be signed in to change notification settings - Fork 5
Refactor to include generic types info in VCs #372
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
Refactor to include generic types info in VCs #372
Conversation
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.
Looks fine to me. @jackastner is the expert though.
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.
There's one part in the FVConstraint
constructor I don't understand. Looks good otherwise.
auto *ParamVar = new PVConstraint(QT, ParmVD, PName, I, Ctx, &N); | ||
int GenericIdx = ParamVar->getGenericIndex(); | ||
if (GenericIdx >= 0) | ||
TypeParams = std::max(TypeParams, GenericIdx + 1); |
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 understand this update to TypeParams
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.
TypeParams
represents the number of variables in Itype_for_any(...)
for a function. Unfortunately, I was unable to read this information directly from the decl. But, each parameter and return var has an index attached, so the max index (+1) is the number of variables. Eventually the rewriter will need to know the location of that phrase in the code, so if you're aware of it let me know. I could make this code slightly more efficient or save the notes for later.
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.
Makes sense. Changes all look good to me then.
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.
Sounds like we are good to go.
Change the generic flag to an index, and have other functions rely on this index rather than regenerating it from decl itypes. This is in preparation for changing these indexes in TypeVariableAnalysis later, so that we can i.e. change
void *
toT *
.All other functionality remains unchanged. Regression tests are unaffected.