From 8d937579737d1da707f08830da1dcaf1f6051ef1 Mon Sep 17 00:00:00 2001 From: Reuben Gardos Reid <5456207+ReubenJ@users.noreply.github.com> Date: Fri, 19 Jan 2024 10:33:38 +0100 Subject: [PATCH 1/2] Add test to show `add_rule!` type coercion bug Currently, the rule `true` is equivalent to the rule `1`. This means that if one of them already exists in the grammar, the new one is skipped. --- test/test_cfg.jl | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/test_cfg.jl b/test/test_cfg.jl index 4b069b1..bdc7880 100644 --- a/test/test_cfg.jl +++ b/test/test_cfg.jl @@ -86,4 +86,28 @@ rm("toy_pcfg.grammar") end + @testset "Test that strict equality is used during rule creation" begin + g₁ = @csgrammar begin + R = x + R = R + R + end + + add_rule!(g₁, :(R = 1 | 2)) + + add_rule!(g₁,:(Bool = true)) + + @test all(g₁.rules .== [:x, :(R + R), 1, 2, true]) + + g₁ = @csgrammar begin + R = x + R = R + R + end + + add_rule!(g₁,:(Bool = true)) + + add_rule!(g₁, :(R = 1 | 2)) + + @test all(g₁.rules .== [:x, :(R + R), true, 1, 2]) + end + end From 5a031ff34259bda69841fefc736f3c74c72c833a Mon Sep 17 00:00:00 2001 From: Reuben Gardos Reid <5456207+ReubenJ@users.noreply.github.com> Date: Fri, 19 Jan 2024 10:34:16 +0100 Subject: [PATCH 2/2] Use strict equality in `add_rule!` Fixes #47. --- src/grammar_base.jl | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/grammar_base.jl b/src/grammar_base.jl index d7be8f9..23a506c 100644 --- a/src/grammar_base.jl +++ b/src/grammar_base.jl @@ -211,13 +211,15 @@ function add_rule!(g::Grammar, e::Expr) rvec = Any[] parse_rule!(rvec, rule) for r ∈ rvec - if r ∈ g.rules - continue + # Only add a rule if it does not exist yet. Check for existance + # with strict equality so that true and 1 are not considered + # equal. that means we can't use `in` or `∈` for equality checking. + if !any(r === rule for rule ∈ g.rules) + push!(g.rules, r) + push!(g.iseval, iseval(rule)) + push!(g.types, s) + g.bytype[s] = push!(get(g.bytype, s, Int[]), length(g.rules)) end - push!(g.rules, r) - push!(g.iseval, iseval(rule)) - push!(g.types, s) - g.bytype[s] = push!(get(g.bytype, s, Int[]), length(g.rules)) end end alltypes = collect(keys(g.bytype))