-
Notifications
You must be signed in to change notification settings - Fork 122
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
Add support for assignments in while-loops #943
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #943 +/- ##
=======================================
Coverage 93.80% 93.80%
=======================================
Files 55 55
Lines 7824 7831 +7
=======================================
+ Hits 7339 7346 +7
Misses 485 485
... and 2 files with indirect coverage changes
|
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.
clang-tidy made some suggestions
cond = cond->IgnoreParenImpCasts(); | ||
auto* condBO = dyn_cast<BinaryOperator>(cond); | ||
auto* condUO = dyn_cast<UnaryOperator>(cond); | ||
// here is determined if condition differentiation is supported |
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.
// here is determined if condition differentiation is supported | |
// FIXME: Currently we only support logical and assignment operators. |
// here is checked if condition is of the form | ||
// ... <logicalOperator> ... |
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.
Worth elaborating. Please write full sentences and maybe give a more complete example.
clang-tidy review says "All clean, LGTM! 👍" |
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.
clang-tidy made some suggestions
clang-tidy review says "All clean, LGTM! 👍" |
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.
clang-tidy made some suggestions
clang-tidy review says "All clean, LGTM! 👍" |
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.
clang-tidy made some suggestions
// | ||
// Visiting statements like "(x=0) || false" records the result in | ||
// condDiff.getExpr(), meaning the differentiated condition is already. | ||
Expr* condInit = condVarClone->getInit(); |
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.
warning: Called C++ object pointer is null [clang-analyzer-core.CallAndMessage]
Expr* condInit = condVarClone->getInit();
^
Additional context
lib/Differentiator/BaseForwardModeVisitor.cpp:1651: 'condVarClone' initialized to a null pointer value
VarDecl* condVarClone = nullptr;
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1659: Assuming 'condVar' is null
if (condVar) {
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1659: Taking false branch
if (condVar) {
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1670: 'cond' is non-null
if (cond) {
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1670: Taking true branch
if (cond) {
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1672: Assuming 'cond' is not a 'CastReturnType'
auto* condBO = dyn_cast<BinaryOperator>(cond);
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1673: Assuming 'cond' is a 'CastReturnType'
auto* condUO = dyn_cast<UnaryOperator>(cond);
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1675: 'condBO' is null
if ((condBO && (condBO->isLogicalOp() || condBO->isAssignmentOp())) ||
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1675: Left side of '&&' is false
if ((condBO && (condBO->isLogicalOp() || condBO->isAssignmentOp())) ||
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1676: 'condUO' is non-null
condUO) {
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1675: Taking true branch
if ((condBO && (condBO->isLogicalOp() || condBO->isAssignmentOp())) ||
^
lib/Differentiator/BaseForwardModeVisitor.cpp:1687: Called C++ object pointer is null
Expr* condInit = condVarClone->getInit();
^
// | ||
// Visiting statements like "(x=0) || false" records the result in | ||
// condDiff.getExpr(), meaning the differentiated condition is already. | ||
Expr* condInit = condVarClone->getInit(); |
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.
warning: Value stored to 'condInit' during its initialization is never read [clang-analyzer-deadcode.DeadStores]
Expr* condInit = condVarClone->getInit();
^
Additional context
lib/Differentiator/BaseForwardModeVisitor.cpp:1687: Value stored to 'condInit' during its initialization is never read
Expr* condInit = condVarClone->getInit();
^
// | ||
// Visiting statements like "(x=0) || false" records the result in | ||
// condDiff.getExpr(), meaning the differentiated condition is already. | ||
Expr* condInit = condVarClone->getInit(); |
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.
warning: unused variable 'condInit' [clang-diagnostic-unused-variable]
Expr* condInit = condVarClone->getInit();
^
This PR adds support for assignments in while-loops. It also enables to combine multiple assignments in the while condition by adding supprot for some logoical operators. Fixes: #913
clang-tidy review says "All clean, LGTM! 👍" |
This PR adds support for assignments in while-loops. It also enables to combine multiple assignments in the while condition by adding support for some logical operators.
Fixes: #913