-
Notifications
You must be signed in to change notification settings - Fork 3
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
Non-redundant ROR mutation for unsigned type #309
base: main
Are you sure you want to change the base?
Conversation
"replacement_operator" wherever appropriate in unary and binary operator mutations, to improve readability.
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.
Looks good. @JamesLee-Jones I would appreciate it if you could check the logic.
@JonathanFoo0523 one minor edit requested before you merge, and it would be great if you could open an issue to capture the opportunity to not even create certain mutations if they would create zero mutants.
new_function | ||
<< " if (__dredd_enabled_mutation(local_mutation_id + " | ||
<< mutation_id_offset << ")) return " << arg1_evaluated << " " |
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.
Not obvious to me what has changed in the above lines, so unsure why the formatting has changed but I assume clang-format has done this, perhaps due to the line below changing. No action needed assuming this is expected.
if (binary_operator_->getLHS()->getType()->isUnsignedIntegerType() && | ||
binary_operator_->getRHS()->getType()->isUnsignedIntegerType()) { |
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.
@JonathanFoo0523 in a previous conversation I think you questioned whether it's possible for the LHS to be unsigned but not the RHS. I don't know for sure, so I'm fine with this being checked the way that it is here.
bool MutationReplaceBinaryOperator::IsRedundantReplacementForUnsignedComparison( | ||
clang::BinaryOperatorKind replacement_operator, | ||
const clang::ASTContext& ast_context) const { | ||
const bool valid_replacement = replacement_operator == clang::BO_NE || |
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.
I don't like the name valid_replacement
as it suggests that if the replacement operator is anything else then the replacement is not valid, whereas this is only used under a given context.
I suggest naming it something mundane like replacement_operator_is_eq_or_ne
.
static int __dredd_replace_binary_operator_NE_arg1_unsigned_int_arg2_unsigned_int_rhs_zero(unsigned int arg1, unsigned int arg2, int local_mutation_id) { | ||
if (!__dredd_some_mutation_enabled) return arg1 != arg2; | ||
return arg1 != arg2; | ||
} |
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.
As an enhancement - for a future issue - we could consider detecting this case in mutate_visitor
and not even creating this function (since it introduces zero mutants). No action for this PR but could you open an issue?
static int __dredd_replace_binary_operator_NE_arg1_unsigned_int_arg2_unsigned_int_lhs_zero(unsigned int arg1, unsigned int arg2, int local_mutation_id) { | ||
if (!__dredd_some_mutation_enabled) return arg1 != arg2; | ||
return arg1 != arg2; | ||
} |
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.
Same.
static int __dredd_replace_binary_operator_LE_arg1_unsigned_int_arg2_unsigned_int_rhs_zero(unsigned int arg1, unsigned int arg2, int local_mutation_id) { | ||
if (!__dredd_some_mutation_enabled) return arg1 <= arg2; | ||
return arg1 <= arg2; | ||
} |
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.
Same.
static int __dredd_replace_binary_operator_GT_arg1_unsigned_int_arg2_unsigned_int_rhs_zero(unsigned int arg1, unsigned int arg2, int local_mutation_id) { | ||
if (!__dredd_some_mutation_enabled) return arg1 > arg2; | ||
return arg1 > arg2; | ||
} |
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.
Same.
static int __dredd_replace_binary_operator_GE_arg1_unsigned_int_arg2_unsigned_int_lhs_zero(unsigned int arg1, unsigned int arg2, int local_mutation_id) { | ||
if (!__dredd_some_mutation_enabled) return arg1 >= arg2; | ||
return arg1 >= arg2; | ||
} |
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.
Same.
static int __dredd_replace_binary_operator_EQ_arg1_unsigned_int_arg2_unsigned_int_rhs_zero(unsigned int arg1, unsigned int arg2, int local_mutation_id) { | ||
if (!__dredd_some_mutation_enabled) return arg1 == arg2; | ||
return arg1 == arg2; | ||
} |
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.
Same.
static int __dredd_replace_binary_operator_EQ_arg1_unsigned_int_arg2_unsigned_int_lhs_zero(unsigned int arg1, unsigned int arg2, int local_mutation_id) { | ||
if (!__dredd_some_mutation_enabled) return arg1 == arg2; | ||
return arg1 == arg2; | ||
} |
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.
Same.
Dredd mutates binary operators to non-redundant operators according to
the rules defined in the referenced paper. Previously, this optimization
allowed for equivalent mutations in the case of unsigned types. A
similar analysis to that described in the paper can be done to
avoid redundant or equivalent mutations for unsigned types.
Non-redundant mutation paper:
https://people.cs.umass.edu/~rjust/publ/non_redundant_mutants_jstvr_2014.pdf
Fixes #240