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 astOperatorNode semantics #1621
Conversation
This reverts commit 9fa5f6d.
Variables are used before checked.
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.
A couple of comments on changes in-line and 2 more since github won't allow commenting on lines not near a change:
Need add two more checks to ast.C lines 2460 & 2464 as both have unprotected loperand->print()
's.
dyninstAPI/src/ast.C
Outdated
@@ -1584,6 +1592,7 @@ bool AstOperatorNode::generateCode_phase2(codeGen &gen, bool noCost, | |||
break; | |||
} | |||
case operandType::RegOffset: { | |||
assert(loperand); |
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.
Remove as redundant. Can loperand
be NULL here? Line 1303 requires it to be not NULL.
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.
No, it can't. I just missed it.
dyninstAPI/src/ast.h
Outdated
@@ -566,6 +572,7 @@ class AstOperatorNode : public AstNode { | |||
|
|||
bool generateOptimizedAssignment(codeGen &gen, int size, bool noCost); | |||
|
|||
AstOperatorNode() {} |
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.
= default
?
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's never used, so I removed it.
It's checked on line 1303.
I fixed them, but that whole definition can be removed. Its declaration was removed in 2006. I think everything guarded by ASTDEBUG can be removed, given how long that's been broken. |
The lack of consistent checking of empty pointers was fixed by #1609, but those changes broke the semantics of the class. This reverts those changes and fixes the pointer checks more directly.