Fix gradient computation of higher order functions #645
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #645 +/- ##
=======================================
Coverage 94.12% 94.12%
=======================================
Files 43 43
Lines 6295 6300 +5
=======================================
+ Hits 5925 5930 +5
Misses 370 370
|
vaithak
force-pushed
the
high-order-fxn
branch
from
c013c5f to
dee9a90
Compare
|
|
|
clang-tidy review says "All clean, LGTM! 👍" |
vaithak
force-pushed
the
high-order-fxn
branch
from
dee9a90 to
72a3890
Compare
|
clang-tidy review says "All clean, LGTM! 👍" |
| addToCurrentBlock(add_assign, direction::reverse); | ||
| // FIXME: not sure if this is generic. | ||
| // Don't update derivatives of non-record types. | ||
| if (!decl->getType()->isRecordType()) { |
There was a problem hiding this comment.
Can we check this with isa<RecordDecl>(decl)?
There was a problem hiding this comment.
I had that earlier but it failed with clang-16. isRecordType does the same thing inside it, but performs the operation on the CanonicalType instead of the qualified type.
source code of isRecordType.
There was a problem hiding this comment.
Ah, ok. That means that if we did isa<RecordDecl>(decl->getCanonicalDecl()) would work?
There was a problem hiding this comment.
Yup, that works. I saw isRecordType being used at different places in our codebase, so I used that.
Should I change this to use isa<RecordDecl>? Any reason to prefer that?
There was a problem hiding this comment.
Nope, let's use your current solution if it is used elsewhere.
fixes #637