Reworked how test defines are implemented to support recursion

Previously test defines were implemented using layers of index-mapped
uintmax_t arrays. This worked well for lookup, but limited defines to
constants computed at compile-time. Since test defines themselves are
actually calculated at _run-time_ (yeah, they have deviated quite
a bit from the original, compile-time evaluated defines, which makes
the name make less sense), this means defines can't depend on other
defines. Which was limiting since a lot of test defines relied on
defines generated from the geometry being tested.

This new implementation uses callbacks for the per-case defines. This
means they can easily contain full C statements, which can depend on
other test defines. This does means you can create infinitely-recursive
defines, but the test-runner will just break at run-time so don't do that.

One concern is that there might be a performance hit for evaluating all
defines through callbacks, but if there is it is well below the noise
floor:

- constants: 43.55s
- callbacks: 42.05s

runners/test_runner.c

@@ -10,34 +10,18 @@
 // test geometries
 struct test_geometry {
     const char *name;
-    test_define_t defines[TEST_GEOMETRY_DEFINE_COUNT];
+    uintmax_t defines[TEST_GEOMETRY_DEFINE_COUNT];
 };
 
 const struct test_geometry test_geometries[TEST_GEOMETRY_COUNT]
         = TEST_GEOMETRIES;
 
 // test define lookup and management
-#define TEST_DEFINE_LAYERS 4
+const uintmax_t *test_override_defines;
-const test_define_t *test_defines[TEST_DEFINE_LAYERS] = {
+uintmax_t (*const *test_case_defines)(void);
-    NULL,
+const uintmax_t *test_geometry_defines;
-    NULL,
+const uintmax_t test_default_defines[TEST_PREDEFINE_COUNT]
-    NULL,
+        = TEST_DEFAULTS;
-    (const test_define_t[TEST_DEFAULT_COUNT])TEST_DEFAULTS,
-};
-
-const uint8_t *test_predefine_maps[TEST_DEFINE_LAYERS] = {
-    NULL,
-    NULL,
-    (const uint8_t[TEST_PREDEFINE_COUNT])TEST_GEOMETRY_DEFINE_MAP,
-    (const uint8_t[TEST_PREDEFINE_COUNT])TEST_DEFAULT_MAP,
-};
-
-const uint8_t *test_define_maps[TEST_DEFINE_LAYERS] = {
-    NULL,
-    NULL,
-    NULL,
-    NULL,
-};
 
 uint8_t test_override_predefine_map[TEST_PREDEFINE_COUNT];
 uint8_t test_override_define_map[256];
@@ -53,28 +37,28 @@ const char *const *test_define_names;
 size_t test_define_count;
 
 
-test_define_t test_predefine(size_t define) {
+uintmax_t test_predefine(size_t define) {
-    for (int i = 0; i < TEST_DEFINE_LAYERS; i++) {
+    if (test_override_defines
-        if (test_defines[i]
+            && test_override_predefine_map[define] != 0xff) {
-                && test_predefine_maps[i]
+        return test_override_defines[test_override_predefine_map[define]];
-                && test_predefine_maps[i][define] != 0xff) {
+    } else if (test_case_defines
-            return test_defines[i][test_predefine_maps[i][define]];
+            && test_case_predefine_map[define] != 0xff
+            && test_case_defines[test_case_predefine_map[define]]) {
+        return test_case_defines[test_case_predefine_map[define]]();
+    } else if (define < TEST_GEOMETRY_DEFINE_COUNT) {
+        return test_geometry_defines[define];
+    } else {
+        return test_default_defines[define-TEST_GEOMETRY_DEFINE_COUNT];
     }
-    }
-
-    fprintf(stderr, "error: undefined predefine %s\n",
-            test_predefine_names[define]);
-    assert(false);
-    exit(-1);
 }
 
-test_define_t test_define(size_t define) {
+uintmax_t test_define(size_t define) {
-    for (int i = 0; i < TEST_DEFINE_LAYERS; i++) {
+    if (test_override_defines
-        if (test_defines[i]
+            && test_override_define_map[define] != 0xff) {
-                && test_define_maps[i]
+        return test_override_defines[test_override_define_map[define]];
-                && test_define_maps[i][define] != 0xff) {
+    } else if (test_case_defines
-            return test_defines[i][test_define_maps[i][define]];
+            && test_case_defines[define]) {
-        }
+        return test_case_defines[define]();
     }
 
     fprintf(stderr, "error: undefined define %s\n",
@@ -84,34 +68,33 @@ test_define_t test_define(size_t define) {
 }
 
 static void test_define_geometry(const struct test_geometry *geometry) {
-    test_defines[2] = geometry->defines;
+    test_geometry_defines = geometry->defines;
 }
 
 static void test_define_overrides(
         const char *const *override_names,
-        const test_define_t *override_defines,
+        const uintmax_t *override_defines,
         size_t override_count) {
-    test_defines[0] = override_defines;
+    test_override_defines = override_defines;
     test_override_names = override_names;
     test_override_count = override_count;
 
-    // map any predefines
+    // map any override predefines
     memset(test_override_predefine_map, 0xff, TEST_PREDEFINE_COUNT);
-    for (size_t i = 0; i < override_count; i++) {
+    for (size_t i = 0; i < test_override_count; i++) {
         for (size_t j = 0; j < TEST_PREDEFINE_COUNT; j++) {
-            if (strcmp(override_names[i], test_predefine_names[j]) == 0) {
+            if (strcmp(test_override_names[i], test_predefine_names[j]) == 0) {
                 test_override_predefine_map[j] = i;
             }
         }
     }
-    test_predefine_maps[0] = test_override_predefine_map;
 }
 
 static void test_define_suite(const struct test_suite *suite) {
     test_define_names = suite->define_names;
     test_define_count = suite->define_count;
 
-    // map any defines
+    // map any override defines
     memset(test_override_define_map, 0xff, suite->define_count);
     for (size_t i = 0; i < test_override_count; i++) {
         for (size_t j = 0; j < suite->define_count; j++) {
@@ -120,26 +103,16 @@ static void test_define_suite(const struct test_suite *suite) {
             }
         }
     }
-    test_define_maps[0] = test_override_define_map;
-}
-
-static void test_define_case(
-        const struct test_suite *suite,
-        const struct test_case *case_) {
-    (void)suite;
-    // case_->define_map is already correct, but we need to do
-    // some fixup for the predefine map
-    test_define_maps[1] = case_->define_map;
 
+    // map any suite/case predefines
     memset(test_case_predefine_map, 0xff, TEST_PREDEFINE_COUNT);
-    for (size_t i = 0; i < test_define_count; i++) {
+    for (size_t i = 0; i < suite->define_count; i++) {
         for (size_t j = 0; j < TEST_PREDEFINE_COUNT; j++) {
-            if (strcmp(test_define_names[i], test_predefine_names[j]) == 0) {
+            if (strcmp(suite->define_names[i], test_predefine_names[j]) == 0) {
-                test_case_predefine_map[j] = case_->define_map[i];
+                test_case_predefine_map[j] = i;
             }
         }
     }
-    test_predefine_maps[1] = test_case_predefine_map;
 }
 
 static void test_define_perm(
@@ -148,9 +121,9 @@ static void test_define_perm(
         size_t perm) {
     (void)suite;
     if (case_->defines) {
-        test_defines[1] = case_->defines[perm];
+        test_case_defines = case_->defines[perm];
     } else {
-        test_defines[1] = NULL;
+        test_case_defines = NULL;
     }
 }
 
@@ -251,7 +224,6 @@ static void summary(void) {
                 continue;
             }
 
-            test_define_case(test_suites[i], test_suites[i]->cases[j]);
             test_case_permcount(test_suites[i], test_suites[i]->cases[j],
                     &perms, &filtered);
         }
@@ -291,7 +263,6 @@ static void list_suites(void) {
                 continue;
             }
 
-            test_define_case(test_suites[i], test_suites[i]->cases[j]);
             test_case_permcount(test_suites[i], test_suites[i]->cases[j],
                     &perms, &filtered);
         }
@@ -325,8 +296,6 @@ static void list_cases(void) {
                 continue;
             }
 
-            test_define_case(test_suites[i], test_suites[i]->cases[j]);
-
             size_t perms = 0;
             size_t filtered = 0;
             test_case_permcount(test_suites[i], test_suites[i]->cases[j],
@@ -379,8 +348,6 @@ static void list_defines(void) {
                 continue;
             }
 
-            test_define_case(test_suites[i], test_suites[i]->cases[j]);
-
             for (size_t perm = 0;
                     perm < TEST_GEOMETRY_COUNT
                         * test_suites[i]->cases[j]->permutations;
@@ -406,9 +373,9 @@ static void list_defines(void) {
 
                 // print each define
                 for (size_t k = 0; k < test_suites[i]->define_count; k++) {
-                    if (test_suites[i]->cases[j]->define_map
+                    if (test_suites[i]->cases[j]->defines
-                            && test_suites[i]->cases[j]->define_map[k]
+                            && test_suites[i]->cases[j]
-                                != 0xff) {
+                                ->defines[case_perm][k]) {
                         printf("%s=%jd ",
                                 test_suites[i]->define_names[k],
                                 test_define(k));
@@ -432,13 +399,11 @@ static void list_geometries(void) {
 
         printf("%-36s ", test_geometries[i].name);
         // print each define
-        for (size_t k = 0; k < TEST_PREDEFINE_COUNT; k++) {
+        for (size_t k = 0; k < TEST_GEOMETRY_DEFINE_COUNT; k++) {
-            if (test_predefine_maps[2][k] != 0xff) {
             printf("%s=%jd ",
                     test_predefine_names[k],
                     test_predefine(k));
         }
-        }
         printf("\n");
 
     }
@@ -447,12 +412,10 @@ static void list_geometries(void) {
 static void list_defaults(void) {
     printf("%-36s ", "defaults");
     // print each define
-    for (size_t k = 0; k < TEST_PREDEFINE_COUNT; k++) {
+    for (size_t k = 0; k < TEST_DEFAULT_DEFINE_COUNT; k++) {
-        if (test_predefine_maps[3][k] != 0xff) {
         printf("%s=%jd ",
-                    test_predefine_names[k],
+                test_predefine_names[k+TEST_GEOMETRY_DEFINE_COUNT],
-                    test_predefine(k));
+                test_predefine(k+TEST_GEOMETRY_DEFINE_COUNT));
-        }
     }
     printf("\n");
 }
@@ -471,8 +434,6 @@ static void run(void) {
                 continue;
             }
 
-            test_define_case(test_suites[i], test_suites[i]->cases[j]);
-
             for (size_t perm = 0;
                     perm < TEST_GEOMETRY_COUNT
                         * test_suites[i]->cases[j]->permutations;
@@ -628,7 +589,7 @@ int main(int argc, char **argv) {
     void (*op)(void) = run;
 
     static const char **override_names = NULL;
-    static test_define_t *override_defines = NULL;
+    static uintmax_t *override_defines = NULL;
     static size_t override_count = 0;
     static size_t override_cap = 0;
 
@@ -730,10 +691,10 @@ int main(int argc, char **argv) {
                     override_names = realloc(override_names, override_cap
                             * sizeof(const char *));
                     override_defines = realloc(override_defines, override_cap
-                            * sizeof(test_define_t));
+                            * sizeof(uintmax_t));
                 }
 
-                // parse into string key/test_define_t value, cannibalizing the
+                // parse into string key/uintmax_t value, cannibalizing the
                 // arg in the process
                 char *sep = strchr(optarg, '=');
                 char *parsed = NULL;

runners/test_runner.h

@@ -12,7 +12,6 @@ enum test_types {
 };
 
 typedef uint8_t test_types_t;
-typedef uintmax_t test_define_t;
 
 struct test_case {
     const char *id;
@@ -21,8 +20,7 @@ struct test_case {
     test_types_t types;
     size_t permutations;
 
-    const test_define_t *const *defines;
+    uintmax_t (*const *const *defines)(void);
-    const uint8_t *define_map;
 
     bool (*filter)(void);
     void (*run)(struct lfs_config *cfg);
@@ -46,8 +44,8 @@ extern const size_t test_suite_count;
 
 
 // access generated test defines
-test_define_t test_predefine(size_t define);
+uintmax_t test_predefine(size_t define);
-test_define_t test_define(size_t define);
+uintmax_t test_define(size_t define);
 
 // a few preconfigured defines that control how tests run
 #define READ_SIZE           test_predefine(0)
@@ -84,10 +82,7 @@ test_define_t test_define(size_t define);
     /* ERASE_CYCLES */      0, \
     /* BADBLOCK_BEHAVIOR */ LFS_TESTBD_BADBLOCK_PROGERROR, \
 }
-#define TEST_DEFAULT_COUNT 5
+#define TEST_DEFAULT_DEFINE_COUNT 5
-#define TEST_DEFAULT_MAP { \
-    0xff, 0xff, 0xff, 0xff, 0xff, 0, 1, 2, 3, 4 \
-}
 
 // test geometries
 #define TEST_GEOMETRIES { \
@@ -100,9 +95,6 @@ test_define_t test_define(size_t define);
 }
 #define TEST_GEOMETRY_COUNT 5
 #define TEST_GEOMETRY_DEFINE_COUNT 5
-#define TEST_GEOMETRY_DEFINE_MAP { \
-    0, 1, 2, 3, 4, 0xff, 0xff, 0xff, 0xff, 0xff \
-}
 
 
 #endif

scripts/test_.py

@@ -68,7 +68,6 @@ class TestCase:
         self.if_ = config.pop('if', None)
         if isinstance(self.if_, bool):
             self.if_ = 'true' if self.if_ else 'false'
-        self.if_lineno = config.pop('if_lineno', None)
         self.code = config.pop('code')
         self.code_lineno = config.pop('code_lineno', None)
         self.in_ = config.pop('in',
@@ -125,18 +124,14 @@ class TestSuite:
             # find line numbers
             f.seek(0)
             case_linenos = []
-            if_linenos = []
             code_linenos = []
             for i, line in enumerate(f):
                 match = re.match(
                     '(?P<case>\[\s*cases\s*\.\s*(?P<name>\w+)\s*\])'
-                        '|' '(?P<if>if\s*=)'
                         '|' '(?P<code>code\s*=)',
                     line)
                 if match and match.group('case'):
                     case_linenos.append((i+1, match.group('name')))
-                elif match and match.group('if'):
-                    if_linenos.append(i+1)
                 elif match and match.group('code'):
                     code_linenos.append(i+2)
 
@@ -147,23 +142,15 @@ class TestSuite:
             for (lineno, name), (nlineno, _) in it.zip_longest(
                     case_linenos, case_linenos[1:],
                     fillvalue=(float('inf'), None)):
-                if_lineno = min(
-                    (l for l in if_linenos if l >= lineno and l < nlineno),
-                    default=None)
                 code_lineno = min(
                     (l for l in code_linenos if l >= lineno and l < nlineno),
                     default=None)
                 cases[name]['lineno'] = lineno
-                cases[name]['if_lineno'] = if_lineno
                 cases[name]['code_lineno'] = code_lineno
 
             self.if_ = config.pop('if', None)
             if isinstance(self.if_, bool):
                 self.if_ = 'true' if self.if_ else 'false'
-            self.if_lineno = min(
-                (l for l in if_linenos
-                    if not case_linenos or l < case_linenos[0][0]),
-                default=None)
 
             self.code = config.pop('code', None)
             self.code_lineno = min(
@@ -274,33 +261,43 @@ def compile(**args):
             # note it's up to the specific generated file to declare
             # the test defines
             def write_case_functions(f, suite, case):
+                    # create case define functions
+                    if case.defines:
+                        # deduplicate defines by value to try to reduce the
+                        # number of functions we generate
+                        define_cbs = {}
+                        for i, defines in enumerate(case.permutations):
+                            for k, v in sorted(defines.items()):
+                                if v not in define_cbs:
+                                    name = ('__test__%s__%s__%s__%d'
+                                        % (suite.name, case.name, k, i))
+                                    define_cbs[v] = name
+                                    f.writeln('uintmax_t %s(void) {' % name)
+                                    f.writeln(4*' '+'return %s;' % v)
+                                    f.writeln('}')
+                                    f.writeln()
+                        f.writeln('uintmax_t (*const *const '
+                            '__test__%s__%s__defines[])(void) = {'
+                            % (suite.name, case.name))
+                        for defines in case.permutations:
+                            f.writeln(4*' '+'(uintmax_t (*const[])(void)){')
+                            for define in sorted(suite.defines):
+                                f.writeln(8*' '+'%s,' % (
+                                    define_cbs[defines[define]]
+                                        if define in defines
+                                        else 'NULL'))
+                            f.writeln(4*' '+'},')
+                        f.writeln('};')
+                        f.writeln()    
+
                     # create case filter function
                     if suite.if_ is not None or case.if_ is not None:
                         f.writeln('bool __test__%s__%s__filter(void) {'
                             % (suite.name, case.name))
-                        if suite.if_ is not None:
+                        f.writeln(4*' '+'return %s;'
-                            if suite.if_lineno is not None:
+                            % ' && '.join('(%s)' % if_
-                                f.writeln(4*' '+'#line %d "%s"'
+                                for if_ in [suite.if_, case.if_]
-                                    % (suite.if_lineno, suite.path))
+                                if if_ is not None))
-                            f.writeln(4*' '+'if (!(%s)) {' % suite.if_)
-                            if suite.if_lineno is not None:
-                                f.writeln(4*' '+'#line %d "%s"'
-                                    % (f.lineno+1, args['output']))
-                            f.writeln(8*' '+'return false;')
-                            f.writeln(4*' '+'}')
-                            f.writeln()
-                        if case.if_ is not None:
-                            if case.if_lineno is not None:
-                                f.writeln(4*' '+'#line %d "%s"'
-                                    % (case.if_lineno, suite.path))
-                            f.writeln(4*' '+'if (!(%s)) {' % case.if_)
-                            if case.if_lineno is not None:
-                                f.writeln(4*' '+'#line %d "%s"'
-                                    % (f.lineno+1, args['output']))
-                            f.writeln(8*' '+'return false;')
-                            f.writeln(4*' '+'}')
-                            f.writeln()
-                        f.writeln(4*' '+'return true;')
                         f.writeln('}')
                         f.writeln()
 
@@ -350,33 +347,14 @@ def compile(**args):
                     f.writeln()
 
                 for case in suite.cases:
-                    # create case defines
-                    if case.defines:
-                        f.writeln('const test_define_t *const '
-                            '__test__%s__%s__defines[] = {'
-                            % (suite.name, case.name))
-                        for permutation in case.permutations:
-                            f.writeln(4*' '+'(const test_define_t[]){%s},'
-                                % ', '.join(str(v) for _, v in sorted(
-                                    permutation.items())))
-                        f.writeln('};')
-                        f.writeln()
-
-                        f.writeln('const uint8_t '
-                            '__test__%s__%s__define_map[] = {'
-                            % (suite.name, case.name))
-                        f.writeln(4*' '+'%s,'
-                            % ', '.join(
-                                str(sorted(case.defines).index(k))
-                                if k in case.defines else '0xff'
-                                for k in sorted(suite.defines)))
-                        f.writeln('};')
-                        f.writeln()
-
                     # create case functions
                     if case.in_ is None:
                         write_case_functions(f, suite, case)
                     else:
+                        if case.defines:
+                            f.writeln('extern uintmax_t (*const *const '
+                                '__test__%s__%s__defines[])(void);'
+                                % (suite.name, case.name))
                         if suite.if_ is not None or case.if_ is not None:
                             f.writeln('extern bool __test__%s__%s__filter('
                                 'void);'
@@ -402,9 +380,6 @@ def compile(**args):
                     if case.defines:
                         f.writeln(4*' '+'.defines = __test__%s__%s__defines,'
                             % (suite.name, case.name))
-                        f.writeln(4*' '+'.define_map = '
-                            '__test__%s__%s__define_map,'
-                            % (suite.name, case.name))
                     if suite.if_ is not None or case.if_ is not None:
                         f.writeln(4*' '+'.filter = __test__%s__%s__filter,'
                             % (suite.name, case.name))