-
Notifications
You must be signed in to change notification settings - Fork 125
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 incorrect derivative produced when array is passed to call expression inside a loop #560
Conversation
…ession inside a loop Added support for passing by reference in a for loop. In VisitArraySubscriptExpr(), for loops , added condition to check if it has reference variable.
My understanding is that's probably not the fix we wanted as many tests seem to fail. I have asked @parth-07 to take a look but since that's inactive for quite some time we might just close it. @ShounakDas101 what do you think? |
@vgvassilev |
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.
Can you please briefly explain your strategy in the commit message?
bool hasReferenceType(const clang::QualType& type) { | ||
return type->isReferenceType(); | ||
} | ||
bool printArgTypes(const clang::CallExpr* CE) { |
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.
Why is this function called printArgTypes
?
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.
It was a prototype function which did print the argument types. I had done this on last day of GSoC application so it is a complete mess and forgot to change the name. This part can be shifted to CladUtils. This function can be simplified a lot. All it is trying to find if there is any ReferenceType. I am very sorry for this.
@@ -1547,10 +1550,15 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context, | |||
block.insert(block.begin() + insertionPoint, | |||
BuildDeclStmt(argDiffLocalVD)); | |||
Expr* argDiffLocalE = BuildDeclRef(argDiffLocalVD); | |||
|
|||
int Numref=0; | |||
while(!m_RPopIdxValues.empty()) |
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.
What does m_RPopIdxValues
represent?
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.
@parth-07 It keeps the DeclStmt of the variables which were being generated because of having a Reference Variable in the argument list but were not being added to the derived function. These variables were being generated in VisitArraySubscriptExpr()
The issues that this pull request intended to fix are already fixed in the master. |
Added support for passing by reference in a for loop. In VisitArraySubscriptExpr(), for loops , added condition to check if it has reference variable. Fixes issue #441 and #465