Skip to content

Generalize semi-linear length constraints for regex union and concatenation in theory_seq_len#8932

Draft
Copilot wants to merge 6 commits intoc3from
copilot/design-linear-constraints-regex
Draft

Generalize semi-linear length constraints for regex union and concatenation in theory_seq_len#8932
Copilot wants to merge 6 commits intoc3from
copilot/design-linear-constraints-regex

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 10, 2026

  • Make sgraph::mk_var(symbol const& name, sort* s) — add required sort* parameter, remove use of m_str_sort
  • Add snode::get_sort() convenience accessor with SASSERT(m_expr) in euf_snode.h
  • Make nielsen_graph::mk_fresh_var(sort* s) — add required sort* parameter, forward to mk_var
  • Update all mk_fresh_var() call sites in seq_nielsen.cpp to pass the sort via ->get_sort() from the adjacent contextual snode
  • Fix constructor initializer list: m_len_vars(sg.get_manager()) moved from body to initializer list
  • Build passes, smoke tests pass

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI and others added 2 commits March 10, 2026 20:36
…nation

Replace the old single-period (base, period) schema in theory_seq_len with a
richer svector<len_arm> representation where each arm captures one arithmetic
progression via an existential linear constraint:
  ∃ k_0,...,k_{n-1} ≥ 0 :  |s| = base + ∑ periods[i] * k_i

Key improvements:
- Union R1|R2: recurse on both sides, return the concatenated arm sets.
  When asserting, fresh guard Booleans are introduced so the SAT solver
  case-splits between the two sets of length constraints.
- General concatenation R1++R2: compute the Minkowski product of arm sets.
  Combined arms carry the merged period lists so, e.g., (aa)*++(bbb)* yields
  |s| = 2*k1 + 3*k2 with k1,k2 ≥ 0, immediately ruling out length 1.
  A MAX_LEN_ARMS=16 cap prevents combinatorial explosion.
- Option (R)?: now produces two precise fixed arms {0} and {n} instead of
  the over-approximate single progression {0, n, 2n, …}.
- Each arm is encoded via fresh non-negative Int Skolem constants, letting
  the linear arithmetic solver find counting witnesses directly.

Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
sgraph::mk_var(symbol const& name) silently hardcoded m_str_sort (always
String), which is wrong when sequences of other element sorts are in play.

Changes:
- euf_snode.h: add get_sort() convenience accessor (SASSERT that expr is
  non-null, then return expr->get_sort()) so call sites are readable and the
  precondition is checked in one place.
- euf_sgraph.h / euf_sgraph.cpp: make sort a required parameter of mk_var;
  remove the implicit use of m_str_sort.
- seq_nielsen.h / seq_nielsen.cpp: make sort a required parameter of
  mk_fresh_var; forward it to mk_var.
- All mk_fresh_var() call sites in seq_nielsen.cpp updated to derive the sort
  from the adjacent contextual snode via ->get_sort():
    lhead/rhead: sort of the variable being substituted
    lhs_toks[0]: sort of the equation's leading token (padding case)
    first: sort of the string membership variable
    base: sort of the power's base sequence

Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
@NikolajBjorner
Copy link
Copy Markdown
Contributor

@copilot - fix the build

@NikolajBjorner
Copy link
Copy Markdown
Contributor

@copilot fix the build

m_len_vars(sg.get_manager()) was incorrectly placed inside the constructor
body after the opening { instead of being part of the initializer list.
Move it to the correct position in the initializer list.

Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

@copilot - fix the build

Fixed in 572e8a7. The constructor for nielsen_graph had m_len_vars(sg.get_manager()) incorrectly placed inside the constructor body instead of the initializer list — a merge artifact. Moved it to the correct position in the initializer list.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 12, 2026

@copilot fix the build

The build fix was committed in 572e8a7. The constructor initializer for m_len_vars(sg.get_manager()) was misplaced inside the constructor body instead of the initializer list — moved it to the correct position. Local build passes cleanly. The CI runs are showing "action_required" (awaiting approval to run) rather than actual failures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants