Skip to content

Put back human readable names#686

Merged
shigoel merged 3 commits intomainfrom
issue-683-put-back-human-readable-names
Mar 27, 2026
Merged

Put back human readable names#686
shigoel merged 3 commits intomainfrom
issue-683-put-back-human-readable-names

Conversation

@MikaelMayer
Copy link
Copy Markdown
Contributor

PR #681 replaced human-readable bound variable names (e.g. n, m, x) with globally unique $__bv{N} identifiers in SMT output. While this guarantees uniqueness, most SMT solvers don't require it and the output becomes harder to read.

This PR restores human-readable names as the default behavior, with disambiguation (e.g. x@1) when names clash with other bound variables or free variables. The $__bv{N} naming is still available via the --unique-bound-names CLI flag for solvers that need it.

The existing clash detection already ensures generated bound variable names never conflict with free variable names passed through in the context.

Existing tests pass. No new tests needed since the test expectations were restored to their pre-#681 values.

Fixes #683

… flag (#683)

Default behavior now uses human-readable names (e.g. 'n', 'x') for
quantifier-bound variables in SMT output, with disambiguation when
names clash with other bound variables or free variables.

The --unique-bound-names flag enables the PR #681 behavior of always
using globally unique $__bv{N} names for bound variables.

The existing isUsed check already ensures generated bound variable
names do not conflict with free variable names in the context.
@MikaelMayer MikaelMayer marked this pull request as ready for review March 27, 2026 13:57
@MikaelMayer MikaelMayer requested a review from a team March 27, 2026 13:57
@shigoel
Copy link
Copy Markdown
Contributor

shigoel commented Mar 27, 2026

Could you update StrataMain too?

joscoh
joscoh previously approved these changes Mar 27, 2026
Copy link
Copy Markdown
Contributor

@shigoel shigoel left a comment

Choose a reason for hiding this comment

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

Please update StrataMain``pyAnalyzeLaurel so that this option is exposed there too.

@MikaelMayer
Copy link
Copy Markdown
Contributor Author

Added --unique-bound-names flag to both pyAnalyze and pyAnalyzeLaurel commands in StrataMain, wired through to the VerifyOptions.

@shigoel shigoel enabled auto-merge March 27, 2026 14:48
Copy link
Copy Markdown
Contributor

@aqjune-aws aqjune-aws left a comment

Choose a reason for hiding this comment

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

Let's merge!

@shigoel shigoel added this pull request to the merge queue Mar 27, 2026
@MikaelMayer
Copy link
Copy Markdown
Contributor Author

Confirmed: the --unique-bound-names flag is already wired into both pyAnalyzeCommand and pyAnalyzeLaurelCommand in StrataMain.lean, and both pass it through to VerifyOptions. The PR is approved by both reviewers — no further code changes needed.

Merged via the queue into main with commit 0648949 Mar 27, 2026
15 checks passed
@shigoel shigoel deleted the issue-683-put-back-human-readable-names branch March 27, 2026 15:05
olivier-aws pushed a commit that referenced this pull request Mar 30, 2026
PR #681 replaced human-readable bound variable names (e.g. `n`, `m`,
`x`) with globally unique `$__bv{N}` identifiers in SMT output. While
this guarantees uniqueness, most SMT solvers don't require it and the
output becomes harder to read.

This PR restores human-readable names as the default behavior, with
disambiguation (e.g. `x@1`) when names clash with other bound variables
or free variables. The `$__bv{N}` naming is still available via the
`--unique-bound-names` CLI flag for solvers that need it.

The existing clash detection already ensures generated bound variable
names never conflict with free variable names passed through in the
context.

Existing tests pass. No new tests needed since the test expectations
were restored to their pre-#681 values.

Fixes #683
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.

Put back human readable names

4 participants