Skip to content

SMT2: disambiguate element-address functions by index type#9064

Open
tautschnig wants to merge 1 commit into
developfrom
strata/smt2-element-address-disambiguation
Open

SMT2: disambiguate element-address functions by index type#9064
tautschnig wants to merge 1 commit into
developfrom
strata/smt2-element-address-disambiguation

Conversation

@tautschnig

Copy link
Copy Markdown
Collaborator

smt2_convt names the helper function emitted for ID_element_address as "element-address-<type2id(result)>", keyed on the result (pointer) type only. When two element_address expressions share a result type but have different index types, both the declaration and the application reuse the same function name with different argument sorts, so an SMT solver rejects the second application with an "unknown constant" sort-mismatch error.

Include type2id(index_type) in the function name in both the application (convert_expr) and the declaration (find_symbols), and build the element size argument in an integer index type when the index type is not itself an integer/bitvector, so the declared and applied sorts always agree.

  • 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.
  • 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).
  • My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • White-space or formatting changes outside the feature-related changed lines are in commits of their own.

smt2_convt names the helper function emitted for ID_element_address as
"element-address-<type2id(result)>", keyed on the result (pointer) type
only. When two element_address expressions share a result type but have
different index types, both the declaration and the application reuse the
same function name with different argument sorts, so an SMT solver rejects
the second application with an "unknown constant" sort-mismatch error.

Include type2id(index_type) in the function name in both the application
(convert_expr) and the declaration (find_symbols), and build the element
size argument in an integer index type when the index type is not itself an
integer/bitvector, so the declared and applied sorts always agree.

Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
@tautschnig tautschnig self-assigned this Jun 18, 2026
Copilot AI review requested due to automatic review settings June 18, 2026 20:02

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.

Disambiguates SMT helper functions for ID_element_address by including the index type in the emitted function name, preventing SMT sort-mismatch errors when the same result type is used with different index types.

Changes:

  • Include type2id(index_type) in the element-address-... helper name for both declaration and application.
  • Ensure the “element size” argument is built with an integer sort (fallback to c_index_type() / unit size as needed) so declared/applied sorts match.

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

Comment on lines +1926 to +1940
// The size/offset arithmetic requires an integer index type. Heap arrays
// keyed by a non-integer type (Strata `Map Ref _`) or with mathematical
// element types have neither a sensible byte size nor an integer index;
// fall back to an integer index type and unit size to avoid building a
// constant of a struct type.
const typet &idx_t = element_address_expr.index().type();
const bool int_index =
idx_t.id() == ID_unsignedbv || idx_t.id() == ID_signedbv ||
idx_t.id() == ID_bv || idx_t.id() == ID_integer ||
idx_t.id() == ID_c_enum || idx_t.id() == ID_c_enum_tag;
const typet size_target = int_index ? idx_t : c_index_type();
const exprt element_size_expr =
element_size_expr_opt.has_value()
? *element_size_expr_opt
: static_cast<exprt>(from_integer(1, size_target));
Comment on lines +1931 to +1936
const typet &idx_t = element_address_expr.index().type();
const bool int_index =
idx_t.id() == ID_unsignedbv || idx_t.id() == ID_signedbv ||
idx_t.id() == ID_bv || idx_t.id() == ID_integer ||
idx_t.id() == ID_c_enum || idx_t.id() == ID_c_enum_tag;
const typet size_target = int_index ? idx_t : c_index_type();
Comment on lines +5992 to +5993
irep_idt function = "element-address-" + type2id(expr.type()) + "_" +
type2id(to_element_address_expr(expr).index().type());
@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 22 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.68%. Comparing base (321ba11) to head (4d2b0a7).

Files with missing lines Patch % Lines
src/solvers/smt2/smt2_conv.cpp 0.00% 22 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #9064      +/-   ##
===========================================
- Coverage    80.68%   80.68%   -0.01%     
===========================================
  Files         1714     1714              
  Lines       189501   189517      +16     
  Branches        73       73              
===========================================
+ Hits        152902   152912      +10     
- Misses       36599    36605       +6     

☔ 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.

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