From 872131a2b1b2c6baf6805503a85da74a4257ca60 Mon Sep 17 00:00:00 2001 From: Scott Vokes Date: Thu, 20 Jul 2023 13:53:30 -0400 Subject: [PATCH 1/3] Re-instate detecting ambiguous mappings in lx. Previously the conflicts were flagged inside of carryopaque, but that was removed in 3d532eef. Now do it in its own pass, called `ast_noteconflicts`. Add a test (`tests/lxconflict/in0.lx` and `out0.err`). --- Makefile | 1 + src/lx/ast.c | 92 +++++++++++++++++++++++++++++++++++++++ src/lx/ast.h | 3 ++ src/lx/main.c | 11 ++--- tests/lxconflict/Makefile | 22 ++++++++++ tests/lxconflict/in0.lx | 1 + tests/lxconflict/out0.err | 1 + 7 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 tests/lxconflict/Makefile create mode 100644 tests/lxconflict/in0.lx create mode 100644 tests/lxconflict/out0.err diff --git a/Makefile b/Makefile index 8d742883e..da6066923 100644 --- a/Makefile +++ b/Makefile @@ -121,6 +121,7 @@ SUBDIR += tests/like SUBDIR += tests/literal # FIXME: commenting this out for now due to Makefile error #SUBDIR += tests/lxpos +SUBDIR += tests/lxconflict SUBDIR += tests/minimise SUBDIR += tests/native SUBDIR += tests/pcre diff --git a/src/lx/ast.c b/src/lx/ast.c index 75754f58d..e6d9234dd 100644 --- a/src/lx/ast.c +++ b/src/lx/ast.c @@ -15,6 +15,7 @@ #include #include +#include #include @@ -268,6 +269,8 @@ ast_getendmapping(const struct fsm *fsm, fsm_state_t s) assert(written == id_count); assert(written > 0); + /* This only returns the first, assuming there is only one. + * More than one end ID would mean conflicts, which are handled separately. */ m = ast_getendmappingbyendid(id_buf[0]); if (LOG()) { @@ -280,3 +283,92 @@ ast_getendmapping(const struct fsm *fsm, fsm_state_t s) } return m; } + +#define DEF_ID_BUF_CEIL 8 +#define DEF_MAPPINGS_CEIL 8 + +int +ast_noteconflicts(const struct fsm *fsm) +{ + const size_t statecount = fsm_countstates(fsm); + size_t s; + size_t id_buf_ceil = DEF_ID_BUF_CEIL; + fsm_end_id_t *id_buf = malloc(id_buf_ceil * sizeof(id_buf[0])); + struct ast_mapping **mappings = malloc(sizeof(mappings[0])); + size_t mappings_ceil = DEF_MAPPINGS_CEIL; + enum fsm_getendids_res getendids_res; + int res = 0; + + if (id_buf == NULL) { + goto cleanup; + } + if (mappings == NULL) { + goto cleanup; + } + + for (s = 0; s < statecount; s++) { + if (!fsm_isend(fsm, s)) { + continue; + } + + const size_t id_count = fsm_getendidcount(fsm, s); + if (id_count == 0) { + continue; + } else if (id_count > id_buf_ceil) { + size_t nceil = 2*id_buf_ceil; + while (nceil < id_count) { + nceil *= 2; + } + fsm_end_id_t *nid_buf = realloc(id_buf, + nceil * sizeof(nid_buf[0])); + if (nid_buf == NULL) { + goto cleanup; + } + id_buf_ceil = nceil; + id_buf = nid_buf; + } + + size_t written; + getendids_res = fsm_getendids(fsm, s, id_count, id_buf, &written); + assert(getendids_res == FSM_GETENDIDS_FOUND); + assert(written == id_count); + assert(written > 0); + + size_t seen = 0; + for (size_t i = 0; i < written; i++) { +#define MAX_SEEN 32 + struct ast_mapping *m = ast_getendmappingbyendid(id_buf[i]); + if (m != NULL) { + if (seen == mappings_ceil) { + size_t nceil = 2 * mappings_ceil; + struct ast_mapping **nmappings = realloc(mappings, + nceil * sizeof(nmappings[0])); + if (nmappings == NULL) { + goto cleanup; + } + mappings = nmappings; + mappings_ceil = nceil; + } + mappings[seen++] = m; + } + } + + /* If more than one mapping is associated with that end state, + * then register the set as conflicts. */ + if (seen > 1) { + struct ast_mapping *m = mappings[0]; + for (size_t i = 0; i < seen; i++) { + if (!ast_addconflict(&m->conflict, + mappings[i])) { + goto cleanup; + } + } + } + } + + res = 1; +cleanup: + free(id_buf); + free(mappings); + return res; +} diff --git a/src/lx/ast.h b/src/lx/ast.h index 097e43841..d63e35f54 100644 --- a/src/lx/ast.h +++ b/src/lx/ast.h @@ -71,5 +71,8 @@ ast_getendmappingbyendid(fsm_end_id_t id); struct ast_mapping * ast_getendmapping(const struct fsm *fsm, fsm_state_t s); +int +ast_noteconflicts(const struct fsm *fsm); + #endif diff --git a/src/lx/main.c b/src/lx/main.c index cbcfeea76..f7ff93637 100644 --- a/src/lx/main.c +++ b/src/lx/main.c @@ -307,14 +307,10 @@ zone_equal(const struct ast_zone *a, const struct ast_zone *b) } { - /* opt.carryopaque = carryopaque; */ - if (!fsm_determinise(q)) { fsm_free(q); return -1; } - - /* opt.carryopaque = NULL; */ } { @@ -842,7 +838,12 @@ main(int argc, char *argv[]) e = 1; } - /* pick up conflicts flagged by carryopaque() */ + if (!ast_noteconflicts(z->fsm)) { + perror("ast_noteconflicts"); + return EXIT_FAILURE; + } + + /* pick up conflicts flagged by ast_noteconflicts() */ for (i = 0; i < z->fsm->statecount; i++) { struct ast_mapping *m; diff --git a/tests/lxconflict/Makefile b/tests/lxconflict/Makefile new file mode 100644 index 000000000..81b3720b6 --- /dev/null +++ b/tests/lxconflict/Makefile @@ -0,0 +1,22 @@ +.include "../../share/mk/top.mk" + +TEST.tests/lxconflict != ls -1 tests/lxconflict/out*.err +TEST_SRCDIR.tests/lxconflict = tests/lxconflict +TEST_OUTDIR.tests/lxconflict = ${BUILD}/tests/lxconflict + +LX?=${BUILD}/bin/lx + +DIR += ${TEST_OUTDIR.tests/lxconflict} + +.for n in ${TEST.tests/lxconflict:T:Mout*.err:R:C/^out//} +${TEST_OUTDIR.tests/lxconflict}/got${n}.err: ${TEST_SRCDIR.tests/lxconflict}/in${n}.lx + ${LX} < ${.ALLSRC:M*.lx} \ + 2> $@; [ $$? -ne 0 ] + +${TEST_OUTDIR.tests/lxconflict}/res${n}: \ + ${TEST_SRCDIR.tests/lxconflict}/out${n}.err \ + ${TEST_OUTDIR.tests/lxconflict}/got${n}.err + +FSMTEST_ERROR += ${TEST_OUTDIR.tests/lxconflict}/res${n} + +.endfor diff --git a/tests/lxconflict/in0.lx b/tests/lxconflict/in0.lx new file mode 100644 index 000000000..e9b427db7 --- /dev/null +++ b/tests/lxconflict/in0.lx @@ -0,0 +1 @@ +/ab?c/ -> $t1; /abc/ -> $t2; \ No newline at end of file diff --git a/tests/lxconflict/out0.err b/tests/lxconflict/out0.err new file mode 100644 index 000000000..040a9ae29 --- /dev/null +++ b/tests/lxconflict/out0.err @@ -0,0 +1 @@ +ambiguous mappings to $t1, $t2; for example on input 'abc' From 6d3ec2908904b2525f1fcfd75d424c464e67edfa Mon Sep 17 00:00:00 2001 From: Scott Vokes Date: Thu, 20 Jul 2023 15:03:26 -0400 Subject: [PATCH 2/3] Fix silly bug, caught by ASan. --- src/lx/ast.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lx/ast.c b/src/lx/ast.c index e6d9234dd..ccb3c9c37 100644 --- a/src/lx/ast.c +++ b/src/lx/ast.c @@ -294,8 +294,8 @@ ast_noteconflicts(const struct fsm *fsm) size_t s; size_t id_buf_ceil = DEF_ID_BUF_CEIL; fsm_end_id_t *id_buf = malloc(id_buf_ceil * sizeof(id_buf[0])); - struct ast_mapping **mappings = malloc(sizeof(mappings[0])); size_t mappings_ceil = DEF_MAPPINGS_CEIL; + struct ast_mapping **mappings = malloc(mappings_ceil * sizeof(mappings[0])); enum fsm_getendids_res getendids_res; int res = 0; From da28efb6ec8a2b8ccc3e9edaa198f665095392ec Mon Sep 17 00:00:00 2001 From: Scott Vokes Date: Thu, 20 Jul 2023 15:55:46 -0400 Subject: [PATCH 3/3] tests/lxconflict: Disable when running CI in (Makefile) mode. We're trying to test lx's error output for ambiguity, so replacing ${LX} with "true; echo lx" obviously won't work, and it also breaks the build in a really confusing way. Just skip it. --- tests/lxconflict/Makefile | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tests/lxconflict/Makefile b/tests/lxconflict/Makefile index 81b3720b6..ca43a968b 100644 --- a/tests/lxconflict/Makefile +++ b/tests/lxconflict/Makefile @@ -1,11 +1,15 @@ .include "../../share/mk/top.mk" +LX?=${BUILD}/bin/lx + +# If this is being run in CI's (Makefiles) mode just ignore this +# test, substituting "true; echo lx" into the ${LX} call below +# obviously won't work. +.if ${LX} != "true; echo lx" TEST.tests/lxconflict != ls -1 tests/lxconflict/out*.err TEST_SRCDIR.tests/lxconflict = tests/lxconflict TEST_OUTDIR.tests/lxconflict = ${BUILD}/tests/lxconflict -LX?=${BUILD}/bin/lx - DIR += ${TEST_OUTDIR.tests/lxconflict} .for n in ${TEST.tests/lxconflict:T:Mout*.err:R:C/^out//} @@ -20,3 +24,4 @@ ${TEST_OUTDIR.tests/lxconflict}/res${n}: \ FSMTEST_ERROR += ${TEST_OUTDIR.tests/lxconflict}/res${n} .endfor +.endif