Skip to content

Extract map_theoryt base class from arrayst#8992

Open
tautschnig wants to merge 3 commits intodiffblue:developfrom
tautschnig:map_theoryt
Open

Extract map_theoryt base class from arrayst#8992
tautschnig wants to merge 3 commits intodiffblue:developfrom
tautschnig:map_theoryt

Conversation

@tautschnig
Copy link
Copy Markdown
Collaborator

@tautschnig tautschnig commented Apr 29, 2026

Separate map-theoretic reasoning (index tracking, equality tracking, Ackermann constraints, read-over-weakeq, extensionality, CDCL(T) propagation, constraint counting) from array-specific encoding (with/if/of/comprehension constraints, lazy selects, 2D inlining).

Inheritance chain: arrayst -> map_theoryt -> equalityt -> prop_conv_solvert

Likely best reviewed commit-by-commit: the second commit introduces a lot of renaming, which impacts call sites. No (intended) functional changes in the second commit.

  • 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 Apr 29, 2026
Copilot AI review requested due to automatic review settings April 29, 2026 17:28
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces a new map_theoryt base class to factor out map-/array-theoretic reasoning infrastructure from arrayst, adjusting the inheritance chain to arrayst -> map_theoryt -> equalityt -> prop_conv_solvert.

Changes:

  • Added map_theoryt (header + constructor TU) containing shared state and helpers for index/equality tracking, Ackermann constraints, and constraint counting.
  • Updated arrayst to inherit from map_theoryt and removed now-shared members from arrays.h.
  • Re-scoped several previously-arrayst helper methods to map_theoryt.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/solvers/flattening/map_theory.h Introduces the new base class API and shared state for map/array reasoning.
src/solvers/flattening/map_theory.cpp Adds map_theoryt constructor implementation.
src/solvers/flattening/arrays.h Switches arrayst to inherit from map_theoryt and removes moved members.
src/solvers/flattening/arrays.cpp Updates implementations to use map_theoryt for extracted functionality.
Comments suppressed due to low confidence (1)

src/solvers/flattening/arrays.cpp:47

  • Several map_theoryt method implementations (e.g., record_array_index, collect_indices, collect_arrays, add_array_constraint, etc.) are now defined in arrays.cpp. This undermines the stated goal of separating map-theoretic reasoning from array-specific encoding and makes linkage/dependencies less clear. Consider moving map_theoryt method definitions into map_theory.cpp and leaving arrays.cpp to only implement arrayst-specific behavior.
void map_theoryt::record_array_index(const index_exprt &index)
{
  // we are not allowed to put the index directly in the
  //   entry for the root of the equivalence class
  //   because this map is accessed during building the error trace
  std::size_t number=arrays.number(index.array());
  if(index_map[number].insert(index.index()).second)
    update_indices.insert(number);
}

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

Comment thread src/solvers/flattening/arrays.h Outdated
Comment thread src/solvers/flattening/arrays.h Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 29, 2026

Codecov Report

❌ Patch coverage is 79.44785% with 67 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.50%. Comparing base (beacad5) to head (52317c2).

Files with missing lines Patch % Lines
src/solvers/flattening/map_theory.cpp 71.59% 50 Missing ⚠️
src/solvers/flattening/arrays.cpp 88.23% 12 Missing ⚠️
src/solvers/flattening/map_theory.h 69.23% 4 Missing ⚠️
src/solvers/flattening/arrays.h 50.00% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #8992      +/-   ##
===========================================
- Coverage    80.50%   80.50%   -0.01%     
===========================================
  Files         1704     1706       +2     
  Lines       188816   188809       -7     
  Branches        73       73              
===========================================
- Hits        152013   152006       -7     
  Misses       36803    36803              

☔ View full report in Codecov by Sentry.
📢 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.

tautschnig and others added 2 commits April 30, 2026 12:02
Separate map-theoretic reasoning (index tracking, equality tracking,
Ackermann constraints, read-over-weakeq, extensionality, CDCL(T)
propagation, constraint counting) from array-specific encoding
(with/if/of/comprehension constraints, lazy selects, 2D inlining).

Inheritance chain: arrayst -> map_theoryt -> equalityt -> prop_conv_solvert

Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
…array'

In a map theory the tracked expressions are 'keys' and their collection
per equivalence class is the 'domain'.  Rename identifiers, type
aliases, function names, and comments accordingly:

  index_sett          → key_sett
  index_mapt          → domain_mapt
  index_map           → domain_map
  update_indices      → update_keys
  collect_indices()   → collect_keys()
  update_index_map()  → update_domain_map()
  record_array_index()→ record_array_key()

Rename all map_theoryt members, methods, types, enum values, and
comments to use map-theory terminology instead of array-specific terms:

  Types:
    array_equalityt         → map_equalityt
    array_equalitiest       → map_equalitiest
    array_constraint_countt → map_constraint_countt

  Members:
    array_equalities        → map_equalities
    arrays (union-find)     → maps
    lazy_array_constraints  → lazy_constraints
    lazy_arrays             → lazy_dispatch
    get_array_constraints   → get_constraints
    array_constraint_count  → constraint_count

  Methods:
    record_array_equality   → record_equality
    record_array_let_binding→ record_let_binding
    record_array_key        → record_key
    collect_arrays          → collect_maps
    is_unbounded_array      → is_unbounded_map
    add_array_constraint    → add_map_constraint
    add_array_Ackermann_constraints → add_Ackermann_constraints
    add_array_constraints_equality  → add_map_equality_constraints
    display_array_constraint_count  → display_constraint_count
    finish_eager_conversion_arrays  → finish_eager_conversion_maps

  Enum values: ARRAY_* → MAP_*

C-level IR names (index_exprt, ID_index, etc.) are unchanged as they
refer to the array-indexing operation, not the map-theory concept.
Array-specific IR names (array_typet, index_exprt, ID_array,
array_comprehension_args, etc.) and arrayst-specific method names
(add_array_constraints, etc.) are unchanged.

Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
Document all public and protected methods, key data members, enums,
and structs in map_theory.h with Doxygen comments following the CBMC
coding standard.

Co-authored-by: Kiro <kiro-agent@users.noreply.github.com>
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