]> Kevux Git Server - fll/commitdiff
Bugfix: If/else condition logic is incorrect or incomplete.
authorKevin Day <thekevinday@gmail.com>
Wed, 12 Jan 2022 02:30:28 +0000 (20:30 -0600)
committerKevin Day <thekevinday@gmail.com>
Wed, 12 Jan 2022 02:30:28 +0000 (20:30 -0600)
The if/else logic is not properly traversing.

I believe the original design is to just have if and else be a single set rather than a chain.

At some point I changed this to be chainable (or maybe I am incorrectly thinking that I did).
It made sense to use this if/then chain logic for my unit test files.
The Fake make unit test files are not working as I want them to because if/else chaining is not working.

As a big disclaimer, this does not support nesting.
I continue to refuse to design nesting of the if/else condition logic as I want to keep this as simple as possible.
Which, by the way, this is not simple.

I attempt to cut corners when I did my recent rewrite of the if/else logic.
Redesign this (again) but this time attempt to do this more properly.

The block process state now exclusively uses a special block process state enumeration rather than relying on a synthetic rule type.
The synthetic rule type is now removed.

The goal with this design is to:
1) On syntax error, bail out of the entire if/else chain.
2) On comparison error, continue an if/else chain passes or the end of chain is reached.
3) When any if/else chain passes, all subsequent conditions in the chain are not run.

Replace the "done" condition result with an "error" condition result to more accurately communicate its use.

Isolate the block and block result from the operate processing as much as possible so that all modifications of the variable are done outside of the operate processing.

When the "fail" is set to "exit" the if/else should fail appropriately on syntax error.
When the "fail" is set to "warn" or "ignore", then continue on after skipping the entire chain.

The operate process still needs to check the if/else chain states to determine if it should skip or not.

When an if/else condition terminates unexpectedly due to an end of file (or section) an error is more consistently reported regardless of place in the if/else chain.

This still needs real testing to confirm that all of the possible permutations work as expected.
Unit tests for this will need to be written but this is a long way away from being done.

level_3/fake/c/private-common.h
level_3/fake/c/private-make-operate.c
level_3/fake/c/private-make-operate_process.c
level_3/fake/c/private-make-operate_process_type.c
level_3/fake/c/private-make-operate_validate.c
level_3/fake/documents/fakefile.txt

index e2c9bcad4be6868a136e0a7911289386958e25c0..3736ceb39d28d83a9be07e240ca4453bd0b1b7e0 100644 (file)
@@ -951,8 +951,7 @@ extern "C" {
   extern const f_string_t fake_make_operation_argument_if_success_s;
 
   enum {
-    fake_make_operation_if_type_done_e = 1,
-    fake_make_operation_if_type_else_e,
+    fake_make_operation_if_type_else_e = 1,
     fake_make_operation_if_type_if_e,
     fake_make_operation_if_type_if_defined_e,
     fake_make_operation_if_type_if_equal_e,
@@ -1172,7 +1171,7 @@ extern "C" {
 /**
  * A structure for managing the operation and if-condition states.
  *
- * block:              The operation type representing the block (either "if" or "else").
+ * block:              The process block state.
  * block_result:       The result of the block.
  * condition:          The current if-condition type.
  * condition_result:   The result of the currently processed condition.
@@ -1200,20 +1199,42 @@ extern "C" {
 #endif // _di_fake_state_process_t_
 
 /**
+ * Designate the current process block state.
+ *
+ * fake_condition_result_*:
+ * - none:    No if-condition is set.
+ * - if:      An if-condition is set and in use.
+ * - if_skip: An else-if-condition that is to be skipped.
+ * - next:    An if-condition completed, process the "else", if found.
+ * - skip:    An if-condition completed, skip the "else", if found.
+ * - else:    An else-condition is set and in use.
+ */
+#ifndef _di_fake_state_process_block_
+  enum {
+    fake_state_process_block_none_e = 0,
+    fake_state_process_block_if_e,
+    fake_state_process_block_if_skip_e,
+    fake_state_process_block_next_e,
+    fake_state_process_block_skip_e,
+    fake_state_process_block_else_e,
+  };
+#endif // _di_fake_state_process_block_
+
+/**
  * Designate that the if state is true, false, or undefined (none).
  *
  * fake_condition_result_*:
  * - none:  The result of the if-condition is not set.
  * - false: The result of the if-condition is false.
  * - true:  The result of the if-condition is true.
- * - done:  The if-condition is done and should no longer be processed.
+ * - error: An error occurred, any if-condition is to be skipped, falling back to the else condition, if any.
  */
 #ifndef _di_fake_condition_result_
   enum {
     fake_condition_result_none_e = 0,
     fake_condition_result_false_e,
     fake_condition_result_true_e,
-    fake_condition_result_done_e,
+    fake_condition_result_error_e,
   };
 #endif // _di_fake_condition_result_
 
index c4019667ca2778350b4d665ebe2d135f3eb07b1f..cde0d7e3056bafeb26786fac408cb1c8ec98e21a 100644 (file)
@@ -1331,9 +1331,8 @@ extern "C" {
         fake_make_operate_validate(data_make, section->name, arguments, &state_process, section_stack, status);
       }
 
-      // Block is set to the synthetic done type when an if-condition completes in order to inform any subsequent else condition.
-      // If there is no follow-up else condition, then reset the block (there is no condition block at this point).
-      if (state_process.block == fake_make_operation_if_type_done_e) {
+      // If next or skip is designated and the next operation is not an "else", then this is no longer in the condition block.
+      if (state_process.block == fake_state_process_block_next_e || state_process.block == fake_state_process_block_skip_e) {
         if (state_process.operation != fake_make_operation_type_else_e) {
           state_process.block = 0;
           state_process.block_result = 0;
@@ -1341,66 +1340,101 @@ extern "C" {
       }
 
       if (F_status_is_error(*status)) {
-        if (state_process.block) {
+        if (state_process.block || state_process.operation == fake_make_operation_type_if_e || state_process.operation == fake_make_operation_type_and_e || state_process.operation == fake_make_operation_type_or_e) {
 
-          // Reset the block information for a completely processed block.
-          if (state_process.block == fake_make_operation_if_type_done_e && state_process.operation != fake_make_operation_type_else_e) {
-            state_process.block = 0;
-            state_process.block_result = 0;
-          }
-
-          // Always set the condition result to done on error for unfinished blocks as there is no way to determine truth at this point.
-          else {
-            state_process.block_result = fake_condition_result_done_e;
+          // Setting error result informs the post-process that the operate process is not run due to an error during the pre-process.
+          if (state_process.operation == fake_make_operation_type_if_e || state_process.operation == fake_make_operation_type_and_e || state_process.operation == fake_make_operation_type_or_e || state_process.operation == fake_make_operation_type_else_e) {
+            state_process.block_result = fake_condition_result_error_e;
           }
         }
       }
       else {
-        if (!(state_process.operation == fake_make_operation_type_if_e || state_process.operation == fake_make_operation_type_else_e || state_process.operation == fake_make_operation_type_and_e || state_process.operation == fake_make_operation_type_or_e)) {
+        result = fake_make_operate_process(data_make, section->name, arguments, success, &state_process, section_stack, status);
 
-          // Only process a non-if-condition operation following an if condition, if the block result is true.
-          if (state_process.block == fake_make_operation_if_type_if_e) {
-            if (state_process.block_result != fake_condition_result_true_e) {
-              state_process.block = fake_make_operation_if_type_done_e;
+        if (*status == F_child) {
+          f_string_dynamics_resize(0, &arguments);
+
+          return result;
+        }
 
-              success = F_true;
+        if (state_process.block && F_status_is_error(result)) {
+          state_process.block_result == fake_condition_result_error_e;
+        }
+      }
 
-              continue;
+      // When done processing an operation within an if-condition or an else-condition, update the process state.
+      if (state_process.block) {
+        if (state_process.block == fake_state_process_block_if_e || state_process.block == fake_state_process_block_if_skip_e) {
+          if (state_process.operation == fake_make_operation_type_if_e || state_process.operation == fake_make_operation_type_else_e) {
+            state_process.block = 0;
+            state_process.block_result = 0;
+          }
+          else if (state_process.operation == fake_make_operation_type_and_e || state_process.operation == fake_make_operation_type_or_e) {
+            if (state_process.block_result == fake_condition_result_error_e) {
+              state_process.block_result = fake_condition_result_false_e;
             }
           }
-
-          // Only process a non-if-condition operation following an else condition, if the block result is false.
-          else if (state_process.block == fake_make_operation_if_type_else_e) {
-            if (state_process.block_result != fake_condition_result_false_e) {
-              state_process.block = 0;
-              state_process.block_result = 0;
-
-              success = F_true;
-
-              continue;
+          else {
+            if (state_process.block == fake_state_process_block_if_skip_e || state_process.block == fake_state_process_block_skip_e) {
+              state_process.block = fake_state_process_block_skip_e;
+              state_process.block_result = fake_condition_result_false_e;
+            }
+            else if (state_process.block_result == fake_condition_result_false_e || state_process.block_result == fake_condition_result_error_e) {
+              state_process.block = fake_state_process_block_next_e;
+              state_process.block_result = fake_condition_result_false_e;
+            }
+            else {
+              state_process.block = fake_state_process_block_skip_e;
             }
           }
         }
+        else if (state_process.block == fake_state_process_block_next_e || state_process.block == fake_state_process_block_skip_e) {
 
-        result = fake_make_operate_process(data_make, section->name, arguments, success, &state_process, section_stack, status);
-
-        if (*status == F_child) {
-          f_string_dynamics_resize(0, &arguments);
+          // Anything other than an "else" operation should not make it here.
+          if (state_process.block == fake_state_process_block_next_e) {
+            state_process.block_result = fake_condition_result_true_e;
+          }
+          else {
+            state_process.block_result = fake_condition_result_false_e;
+          }
 
-          return result;
+          state_process.block = fake_state_process_block_else_e;
         }
+        else if (state_process.block == fake_state_process_block_else_e) {
 
-        // When done processing an operation within a block, continue onto next block stage or exit the block.
-        if (!(state_process.operation == fake_make_operation_type_if_e || state_process.operation == fake_make_operation_type_else_e || state_process.operation == fake_make_operation_type_and_e || state_process.operation == fake_make_operation_type_or_e)) {
-          if (state_process.block == fake_make_operation_if_type_if_e) {
-            state_process.block = fake_make_operation_if_type_done_e;
+          // An "else" followed by an "if" resets the state back to an "if".
+          if (state_process.operation == fake_make_operation_type_if_e) {
+            if (state_process.block_result == fake_condition_result_true_e) {
+              state_process.block = fake_state_process_block_if_e;
+              state_process.block_result = state_process.condition_result;
+            }
+            else {
+              state_process.block = fake_state_process_block_if_skip_e;
+              state_process.block_result == fake_condition_result_false_e;
+            }
           }
-          else if (state_process.block == fake_make_operation_if_type_else_e) {
+          else {
             state_process.block = 0;
             state_process.block_result = 0;
           }
         }
       }
+      else if (state_process.operation == fake_make_operation_type_if_e || state_process.operation == fake_make_operation_type_and_e || state_process.operation == fake_make_operation_type_or_e) {
+        if (state_process.block_result == fake_condition_result_error_e) {
+          state_process.block = fake_state_process_block_if_skip_e;
+          state_process.block_result = fake_condition_result_false_e;
+        }
+        else {
+          state_process.block = fake_state_process_block_if_e;
+          state_process.block_result = state_process.condition_result;
+        }
+      }
+      else if (state_process.operation == fake_make_operation_type_else_e) {
+
+        // An else condition by itself is invalid and the operation associated with it should be skipped.
+        state_process.block = fake_state_process_block_else_e;
+        state_process.block_result = fake_condition_result_false_e;
+      }
 
       if (F_status_set_fine(*status) == F_interrupt) break;
 
@@ -1464,23 +1498,17 @@ extern "C" {
       return 0;
     }
 
-    if (i == section->objects.used && F_status_is_error_not(*status) && state_process.condition) {
+    if (i == section->objects.used && F_status_is_error_not(*status) && (state_process.block == fake_state_process_block_if_e || state_process.block == fake_state_process_block_if_skip_e || state_process.block == fake_state_process_block_else_e)) {
       if (data_make->main->error.verbosity != f_console_verbosity_quiet_e && data_make->error.to.stream) {
         flockfile(data_make->error.to.stream);
 
         fl_print_format("%c%[%SIncomplete '%]", data_make->error.to.stream, f_string_eol_s[0], data_make->error.context, data_make->error.prefix, data_make->error.context);
 
-        if (state_process.condition == fake_make_operation_type_if_e) {
-          fl_print_format("%[%s%]", data_make->error.to.stream, data_make->error.notable, fake_make_operation_if_s, data_make->error.notable);
-        }
-        else if (state_process.condition == fake_make_operation_type_and_e) {
-          fl_print_format("%[%s%]", data_make->error.to.stream, data_make->error.notable, fake_make_operation_and_s, data_make->error.notable);
-        }
-        else if (state_process.condition == fake_make_operation_type_or_e) {
-          fl_print_format("%[%s%]", data_make->error.to.stream, data_make->error.notable, fake_make_operation_or_s, data_make->error.notable);
+        if (state_process.block == fake_state_process_block_else_e) {
+          fl_print_format("%[%s%]", data_make->error.to.stream, data_make->error.notable, fake_make_operation_else_s, data_make->error.notable);
         }
         else {
-          fl_print_format("%[%s%]", data_make->error.to.stream, data_make->error.notable, fake_make_operation_else_s, data_make->error.notable);
+          fl_print_format("%[%s%]", data_make->error.to.stream, data_make->error.notable, fake_make_operation_if_s, data_make->error.notable);
         }
 
         fl_print_format("%[' at end of section.%]%c", data_make->error.to.stream, data_make->error.context, data_make->error.context, f_string_eol_s[0]);
index 443ce2efdfa8a81ea75cb643b2ac5027eeb47573..4fca8f0b38c3ae8436b0af895b2b7da7d8ce1e17 100644 (file)
@@ -19,6 +19,28 @@ extern "C" {
 
     if (*status == F_child) return data_make->main->child;
 
+    if (state_process->block) {
+      if (state_process->block == fake_state_process_block_if_e) {
+        if (!(state_process->operation == fake_make_operation_type_and_e || state_process->operation == fake_make_operation_type_or_e)) {
+          if (state_process->block_result == fake_condition_result_false_e) {
+            return 0;
+          }
+        }
+      }
+      else if (state_process->block == fake_state_process_block_if_skip_e) {
+        if (!(state_process->operation == fake_make_operation_type_and_e || state_process->operation == fake_make_operation_type_or_e)) {
+          return 0;
+        }
+      }
+      else if (state_process->block == fake_state_process_block_else_e) {
+        if (state_process->operation != fake_make_operation_type_if_e) {
+          if (state_process->block_result == fake_condition_result_false_e) {
+            return 0;
+          }
+        }
+      }
+    }
+
     if (state_process->operation == fake_make_operation_type_index_e) {
       const f_status_t result = fake_execute(data_make->main, data_make->environment, data_make->setting_build.build_indexer, arguments, status);
 
@@ -146,9 +168,7 @@ extern "C" {
 
     if (state_process->operation == fake_make_operation_type_else_e) {
 
-      // Setup the block so that subsequent operations can know the current block and the result (which is preserved across to the else condition).
-      state_process->block = fake_make_operation_if_type_else_e;
-
+      // There is nothing to do.
       return 0;
     }
 
@@ -286,26 +306,30 @@ extern "C" {
         *status = fake_make_operate_process_type_if_owner(data_make, arguments, F_false, state_process);
       }
 
-      // Setup the block for subsequent operations.
-      state_process->block = fake_make_operation_if_type_if_e;
+      // When inside an else block that is already set to false, then all if conditions inside are always considered false.
+      if (state_process->block == fake_state_process_block_else_e && state_process->block_result == fake_condition_result_false_e) {
+        return 0;
+      }
 
-      if (state_process->condition_result == fake_condition_result_done_e || state_process->operation == fake_make_operation_type_if_e) {
-        state_process->block_result = state_process->condition_result;
+      // When inside an if block designated to be skipped, then all if conditions inside are always considered false.
+      if (state_process->block == fake_state_process_block_if_skip_e) {
+        return 0;
       }
-      else if (state_process->operation == fake_make_operation_type_or_e) {
+
+      if (state_process->operation == fake_make_operation_type_or_e) {
         if (state_process->block_result == fake_condition_result_true_e || state_process->condition_result == fake_condition_result_true_e) {
-          state_process->block_result = fake_condition_result_true_e;
+          state_process->condition_result = fake_condition_result_true_e;
         }
         else {
-          state_process->block_result = fake_condition_result_false_e;
+          state_process->condition_result = fake_condition_result_false_e;
         }
       }
-      else {
+      else if (state_process->block) {
         if (state_process->block_result == fake_condition_result_true_e && state_process->condition_result == fake_condition_result_true_e) {
-          state_process->block_result = fake_condition_result_true_e;
+          state_process->condition_result = fake_condition_result_true_e;
         }
         else {
-          state_process->block_result = fake_condition_result_false_e;
+          state_process->condition_result = fake_condition_result_false_e;
         }
       }
 
index 179ccebca3f0437308a0c28842420ed5d04914d9..f6d5ef8f9711f6d09c8f3d04a9e607129b75353f 100644 (file)
@@ -521,7 +521,7 @@ extern "C" {
       status = f_file_exists(arguments.array[i].string);
 
       if (F_status_is_error(status)) {
-        state_process->condition_result = fake_condition_result_done_e;
+        state_process->condition_result = fake_condition_result_error_e;
 
         fll_error_file_print(data_make->error, F_status_set_fine(status), "f_file_exists", F_true, arguments.array[i].string, "find", fll_error_file_type_file_e);
 
@@ -606,7 +606,7 @@ extern "C" {
       status = f_file_mode_read(arguments.array[i].string, &mode_file);
 
       if (F_status_is_error(status)) {
-        state_process->condition_result = fake_condition_result_done_e;
+        state_process->condition_result = fake_condition_result_error_e;
 
         fll_error_file_print(data_make->error, F_status_set_fine(status), "f_file_mode_read", F_true, arguments.array[i].string, "get mode of", fll_error_file_type_file_e);
 
@@ -786,7 +786,7 @@ extern "C" {
     }
 
     if (F_status_is_error(status)) {
-      state_process->condition_result = fake_condition_result_done_e;
+      state_process->condition_result = fake_condition_result_error_e;
 
       if (data_make->main->error.verbosity != f_console_verbosity_quiet_e && data_make->error.to.stream) {
         flockfile(data_make->error.to.stream);
@@ -830,7 +830,7 @@ extern "C" {
       status = f_file_group_read(arguments.array[i].string, &id_file);
 
       if (F_status_is_error(status)) {
-        state_process->condition_result = fake_condition_result_done_e;
+        state_process->condition_result = fake_condition_result_error_e;
 
         fll_error_file_print(data_make->error, F_status_set_fine(status), "f_file_group_read", F_true, arguments.array[i].string, "get group of", fll_error_file_type_file_e);
 
@@ -877,7 +877,7 @@ extern "C" {
       status = fake_make_get_id_mode(data_make->main, data_make->error, arguments.array[if_not ? 3 : 2], &mode_rule, &mode_replace);
 
       if (F_status_is_error(status)) {
-        state_process->condition_result = fake_condition_result_done_e;
+        state_process->condition_result = fake_condition_result_error_e;
 
         return status;
       }
@@ -885,7 +885,7 @@ extern "C" {
       status = f_file_mode_to_mode(mode_rule, &mode_match);
 
       if (F_status_is_error(status)) {
-        state_process->condition_result = fake_condition_result_done_e;
+        state_process->condition_result = fake_condition_result_error_e;
 
         fll_error_print(data_make->error, F_status_set_fine(status), "f_file_mode_to_mode", F_true);
 
@@ -902,7 +902,7 @@ extern "C" {
       status = f_file_mode_read(arguments.array[i].string, &mode_file);
 
       if (F_status_is_error(status)) {
-        state_process->condition_result = fake_condition_result_done_e;
+        state_process->condition_result = fake_condition_result_error_e;
 
         fll_error_file_print(data_make->error, F_status_set_fine(status), "f_file_mode_read", F_true, arguments.array[i].string, "get mode of", fll_error_file_type_file_e);
 
@@ -965,7 +965,7 @@ extern "C" {
       status = f_file_owner_read(arguments.array[i].string, &id_file);
 
       if (F_status_is_error(status)) {
-        state_process->condition_result = fake_condition_result_done_e;
+        state_process->condition_result = fake_condition_result_error_e;
 
         fll_error_file_print(data_make->error, F_status_set_fine(status), "f_file_owner_read", F_true, arguments.array[i].string, "get owner of", fll_error_file_type_file_e);
 
index 2d7d902685086d79a10ce7b9dc7c53481c4a16d8..7fb647803ac7d1f1804526a06760ea4d1099d5f3 100644 (file)
@@ -445,7 +445,7 @@ extern "C" {
         return;
       }
 
-      if (state_process->block != fake_make_operation_if_type_done_e) {
+      if (!state_process->block) {
         if (data_make->error.verbosity != f_console_verbosity_quiet_e && data_make->error.to.stream) {
           fl_print_format("%c%[%SHas no preceding '%]", data_make->error.to.stream, f_string_eol_s[0], data_make->error.context, data_make->error.prefix, data_make->error.context);
           fl_print_format("%[%s%]", data_make->error.to.stream, data_make->error.notable, fake_make_operation_if_s, data_make->error.notable);
@@ -568,7 +568,7 @@ extern "C" {
 
     if (state_process->operation == fake_make_operation_type_if_e || state_process->operation == fake_make_operation_type_and_e || state_process->operation == fake_make_operation_type_or_e) {
       if (state_process->operation == fake_make_operation_type_if_e) {
-        if (state_process->block == fake_make_operation_type_if_e) {
+        if (state_process->operation_previous == fake_make_operation_type_if_e) {
           if (data_make->error.verbosity != f_console_verbosity_quiet_e && data_make->error.to.stream) {
             flockfile(data_make->error.to.stream);
 
index cca780bc46ae692d5177d1ea0265aa6c67cd9910..4a0c719c16c851bf8344e44546d734b275747652 100644 (file)
@@ -42,7 +42,7 @@ Fakefile Documentation:
       - warn: Designates to continue on and if in verbose mode then print a warning.
       - ignore: Designates to do nothing.
 
-      The return code for programs can still be retrieved through using the reserved iki vaiable "return".
+      The return code for programs can still be retrieved through using the reserved IKI vaiable "return".
 
     - indexer\:
       This represents the name of the indexer program to use, such as "ar".