Skip to content

SMT2: only typecast array index for fixed-width bitvector index types#9073

Open
tautschnig wants to merge 1 commit into
diffblue:developfrom
tautschnig:strata/smt2-array-index-typecast
Open

SMT2: only typecast array index for fixed-width bitvector index types#9073
tautschnig wants to merge 1 commit into
diffblue:developfrom
tautschnig:strata/smt2-array-index-typecast

Conversation

@tautschnig

@tautschnig tautschnig commented Jun 18, 2026

Copy link
Copy Markdown
Collaborator

When converting array store/select, typecast the index to the array's index type only when both are fixed-width bitvectors; for mathematical-integer or struct-typed (map key) indices, emit the index directly.

  • Each commit message has a non-empty body, explaining why the change was made.
  • Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@tautschnig tautschnig self-assigned this Jun 18, 2026
Copilot AI review requested due to automatic review settings June 18, 2026 21:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adjust SMT2 array store/select index handling to avoid unnecessary typecasts, only casting when both index types are fixed-width bitvectors.

Changes:

  • Conditionally typecast array store indices only when both sides are fixed-width bitvector-like types.
  • Conditionally typecast array select (index) expressions using the same rule, emitting integer/struct indices directly.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/solvers/smt2/smt2_conv.cpp Outdated
Comment thread src/solvers/smt2/smt2_conv.cpp Outdated
Comment thread src/solvers/smt2/smt2_conv.cpp Outdated
Comment thread src/solvers/smt2/smt2_conv.cpp Outdated
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.91667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 80.69%. Comparing base (7483d0d) to head (00d968c).

Files with missing lines Patch % Lines
src/solvers/smt2/smt2_conv.cpp 93.75% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #9073   +/-   ##
========================================
  Coverage    80.68%   80.69%           
========================================
  Files         1714     1714           
  Lines       189593   189638   +45     
  Branches        73       73           
========================================
+ Hits        152979   153024   +45     
  Misses       36614    36614           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

When converting an array store/select, the index was unconditionally
typecast to the array's index type, which is wrong once that index type is a
mathematical integer or a struct (e.g. a map key): the cast is unnecessary
(the sorts already match) and, for struct keys, ill-formed.

Coerce the index to the array's declared index type only when that type is a
fixed-width bitvector and differs from the index expression's type; otherwise
emit the index unchanged.  The decision is keyed on the *destination* (array)
index type so that an integer- or differently-sized-bitvector-sourced index is
still coerced when the array's index type is a bitvector -- in particular when
array_typet::index_type() falls back to c_index_type() for an array without an
ID_C_index_type annotation, which would otherwise feed an integer-sorted index
into a bitvector-sorted store/select (ill-sorted SMT).  A DATA_INVARIANT makes
the remaining assumption explicit: a bitvector index requires the array's
index type to be a bitvector too.

The selection logic (previously duplicated verbatim in convert_with and
convert_index) is factored into a single cast_array_index() helper.  Enum and
enum-tag index types are treated as bitvector-like; convert_typecast resolves
an enum-tag destination via ns.follow_tag, so the emission stays valid.

A unit test (unit/solvers/smt2/smt2_conv.cpp) pins the serialization for all
three cases -- mathematical-integer index, matching-width bitvector index, and
differing-width bitvector index -- for both select and store.

Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
@tautschnig tautschnig force-pushed the strata/smt2-array-index-typecast branch from 326cd10 to 00d968c Compare June 24, 2026 13:08
@tautschnig tautschnig assigned kroening and unassigned tautschnig Jun 24, 2026
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.

3 participants