Skip to content
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

Fix passing pointers as call arguments #763

Merged
merged 1 commit into from
Feb 15, 2024

Conversation

vaithak
Copy link
Collaborator

@vaithak vaithak commented Feb 15, 2024

fixes #735, #636

@vgvassilev
Copy link
Owner

Can you check if this PR fixes #762 by any chance?

Copy link

codecov bot commented Feb 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9f3556d) 94.70% compared to head (3347684) 94.71%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #763      +/-   ##
==========================================
+ Coverage   94.70%   94.71%   +0.01%     
==========================================
  Files          49       49              
  Lines        7322     7342      +20     
==========================================
+ Hits         6934     6954      +20     
  Misses        388      388              
Files Coverage Δ
include/clad/Differentiator/CladUtils.h 100.00% <ø> (ø)
include/clad/Differentiator/ReverseModeVisitor.h 97.87% <ø> (ø)
lib/Differentiator/CladUtils.cpp 97.14% <100.00%> (+0.03%) ⬆️
lib/Differentiator/ReverseModeVisitor.cpp 96.38% <100.00%> (+0.02%) ⬆️
Files Coverage Δ
include/clad/Differentiator/CladUtils.h 100.00% <ø> (ø)
include/clad/Differentiator/ReverseModeVisitor.h 97.87% <ø> (ø)
lib/Differentiator/CladUtils.cpp 97.14% <100.00%> (+0.03%) ⬆️
lib/Differentiator/ReverseModeVisitor.cpp 96.38% <100.00%> (+0.02%) ⬆️

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

clang-tidy made some suggestions

lib/Differentiator/ReverseModeVisitor.cpp Outdated Show resolved Hide resolved
@vaithak
Copy link
Collaborator Author

vaithak commented Feb 15, 2024

Can you check if this PR fixes #762 by any chance?

That is because of a separate reason, commented on that issue.

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

@@ -1536,7 +1551,8 @@ Expr* getArraySizeExpr(const ArrayType* AT, ASTContext& context,
// FIXME: We cannot use GlobalStoreAndRef to store a whole array so now
// arrays are not stored.
StmtDiff argDiffStore;
if (passByRef && !argDiff.getExpr()->getType()->isArrayType())
if (passByRef && !argDiff.getExpr()->getType()->isArrayType() &&
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we use argDiffType here, too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was inside the if condition earlier, but moved it out now 👍🏼

Copy link
Contributor

clang-tidy review says "All clean, LGTM! 👍"

Copy link
Owner

@vgvassilev vgvassilev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@parth-07
Copy link
Collaborator

@vaithak Can you please update pull request description / commit message with details how it addresses the issue?

Does this pull request addresses pointer arithmetic expressions such as p + 1 used as function call arguments? If so, can you please add a test?

Also, wouldn't this pull request #762 fix the issue as well?

@vgvassilev vgvassilev merged commit d305002 into vgvassilev:master Feb 15, 2024
81 checks passed
@vgvassilev
Copy link
Owner

@parth-07, I did not see your comment before merging. I guess I was too excited about this PR. Apologies!

@vaithak
Copy link
Collaborator Author

vaithak commented Feb 15, 2024

@vaithak Can you please update pull request description / commit message with details how it addresses the issue?

Does this pull request addresses pointer arithmetic expressions such as p + 1 used as function call arguments? If so, can you please add a test?

Also, wouldn't this pull request #762 fix the issue as well?

For #762, I have commented more about that on the issue itself. Regarding pointer arithmetic, I will test it out and make changes if required along with test cases in a separate PR.

@vaithak vaithak deleted the nullptr branch March 13, 2024 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants