Fix pointer dereference in fwd mode #975
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
clang-tidy review says "All clean, LGTM! 👍" |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #975 +/- ##
=======================================
Coverage 93.94% 93.95%
=======================================
Files 55 55
Lines 7962 7967 +5
=======================================
+ Hits 7480 7485 +5
Misses 482 482
|
|
clang-tidy review says "All clean, LGTM! 👍" |
| if (clonedDRE->getType()->isPointerType()) | ||
| return StmtDiff(clonedDRE, nullptr); | ||
| return StmtDiff(clonedDRE, ConstantFolder::synthesizeLiteral( | ||
| m_Context.IntTy, m_Context, /*val=*/0)); |
There was a problem hiding this comment.
| if (clonedDRE->getType()->isPointerType()) | |
| return StmtDiff(clonedDRE, nullptr); | |
| return StmtDiff(clonedDRE, ConstantFolder::synthesizeLiteral( | |
| m_Context.IntTy, m_Context, /*val=*/0)); | |
| QualType clonedDreTy = clonedDRE->getType(); | |
| if (clonedDreTy->isPointerType()) | |
| return StmtDiff(clonedDRE, nullptr); | |
| return StmtDiff(clonedDRE, ConstantFolder::synthesizeLiteral( | |
| clonedDreTy, m_Context, /*val=*/0)); |
| Expr* dx = diff.getExpr_dx(); | ||
| if (dx == nullptr) | ||
| return StmtDiff(op, ConstantFolder::synthesizeLiteral( | ||
| m_Context.IntTy, m_Context, /*val=*/0)); | ||
| return StmtDiff(op, BuildOp(opKind, dx)); |
There was a problem hiding this comment.
| Expr* dx = diff.getExpr_dx(); | |
| if (dx == nullptr) | |
| return StmtDiff(op, ConstantFolder::synthesizeLiteral( | |
| m_Context.IntTy, m_Context, /*val=*/0)); | |
| return StmtDiff(op, BuildOp(opKind, dx)); | |
| if (Expr* dx = diff.getExpr_dx()) | |
| return StmtDiff(op, BuildOp(opKind, dx)); | |
| return StmtDiff(op, ConstantFolder::synthesizeLiteral( | |
| m_Context.IntTy, m_Context, /*val=*/0)); |
I know that we are not using this consistently but IntTy here is probably not what we want semantically. Shall we give it a try with a VoidPtrTy or a NullPtrTy?
There was a problem hiding this comment.
It can't be a ptr type, as we want a numeric type here. So, changed to use the pointee type essentially 👍🏼
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
| if (!clonedDreTy->isRealType()) | ||
| clonedDreTy = m_Context.IntTy; // force int if not real, ex. custom type |
There was a problem hiding this comment.
Should we fix the implementation of ConstantFolder::synthesizeLiteral to generate maybe {} initializer list which is supposed to call the default constructor of the custom type?
PS: Sorry for making this hard as that's not quite related to this PR...
There was a problem hiding this comment.
It wouldn't be a good idea.
It can be done, but we will be assuming that a default constructor exists for this custom type, which properly zero initializes the data members. This assumption may constrain what we can differentiate.
There was a problem hiding this comment.
Sure, but how this is different from what we have now scattered all over the place? I guess I was proposing to centralize the logic there and if the type does not have a default constructor with proper initialization we can complain...
|
clang-tidy review says "All clean, LGTM! 👍" |
|
Maybe we should revert the PR to the old state where everything passed, merge it and continue with this change separately. That would unblock the release. |
Updated 👍🏼 |
|
clang-tidy review says "All clean, LGTM! 👍" |
fixes #972