Mutant testing: No test fails if preprocessing condition is reversed

[hzeller]

Consider the following: here we change the value of if_condition to mean the reverse.

--- a/verilog/analysis/flow_tree.cc
+++ b/verilog/analysis/flow_tree.cc
@@ -192,7 +192,7 @@ absl::Status FlowTree::GenerateControlFlowTree() {
         case PP_ifdef: {
           if_blocks_.push_back(empty_block);
           if_blocks_.back().if_location = iter;
-          if_blocks_.back().positive_condition = 1;
+          if_blocks_.back().positive_condition = 0;
           auto status = AddMacroOfConditional(iter);
           if (!status.ok()) {
             return absl::InvalidArgumentError(
@@ -203,7 +203,7 @@ absl::Status FlowTree::GenerateControlFlowTree() {
         case PP_ifndef: {
           if_blocks_.push_back(empty_block);
           if_blocks_.back().if_location = iter;
-          if_blocks_.back().positive_condition = 0;
+          if_blocks_.back().positive_condition = 1;
           auto status = AddMacroOfConditional(iter);
           if (!status.ok()) {
             return absl::InvalidArgumentError(

Running all tests still pass afterwards. So existing tests do not probe enough to discover it.

[hzeller]

CC: @karimtera

[karimtera]

Yes, I noticed this when I was creating tests for #1372.

There is a reason behind this, so ConditionalBlock is used only in FlowTree::AddBlockEdges(), and I follow a convention in these edges, since for each iterator in the conditional block we would have 2 edges at most, I fixed these 2 edges as the following:

• The first edge always means that the condition is true (whatever that condition is ifdef/ifndef).
• The second edge refers to the condition being false.

Now, it would be easier for me to add edges, but there will be more logic done in DepthFirstSearch(), as I would need to set/reset the bitsets meaning that I need to translate "a condition being true/false" to "a macro is defined/undefined".

So, it is just a choice where to do things, but I agree that in my use model, I don't use positive_condition at all.

[hzeller]

Alright, so at the decision points you essentially look at the if_position to determine if this is an ifdef or ifndef ?
Then we can safely remove the positive_condition value.

Going to do that in #1431

[hzeller]

In the edge generation, the first condition is assumed true according to the comment, and there does not seem to be anything that re-arranges things differently if the if_position is pointing to an ifndef.

verible/verilog/analysis/flow_tree.cc

Line 37 in 0305dd4

// Assuming the condition is true.

It looks like it is happening in the DepthFirstSearch()

verible/verilog/analysis/flow_tree.cc

Line 290 in 0305dd4

bool negated = (current_node->token_enum() == PP_ifndef);

[karimtera]

Yes, exactly it is happening in DepthFirstSearch(), where ConditionalBlock doesn't exist.