Skip to content

Ensure ssa_expr's l1-identifier is consistent with its identifier [blocks: #2574]#3719

Merged
tautschnig merged 1 commit intodiffblue:developfrom
tautschnig:ssa-field-sensitivity-l1
Jan 9, 2019
Merged

Ensure ssa_expr's l1-identifier is consistent with its identifier [blocks: #2574]#3719
tautschnig merged 1 commit intodiffblue:developfrom
tautschnig:ssa-field-sensitivity-l1

Conversation

@tautschnig
Copy link
Collaborator

@tautschnig tautschnig commented Jan 8, 2019

The new cases for ssa-expressions describing members or indices didn't contribute to the l1-id
as they ought to.

This is a bugfix for a code path not currently executed. #2574 will bring it to life.

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a 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.

@smowton @tautschnig
Ensure ssa_expr's l1-identifier is consistent with its identifier
cedceae 
The new cases for ssa-expressions describing members or indices didn't contribute to the l1-id
as they ought to.
@smowton
Copy link
Contributor

smowton commented Jan 8, 2019

  1. Do you want to go for the route of using a distinguished character(s) from the usual index or member operator, so as to aid reading the difference between such a symbol and an operator when it pretty-printed form (and to prevent accidental aliasing when using strings as keys)?
  2. Certainly in the current field-sensitivity work the version identifiers are significant on the granularity of an SSA expression, so e.g. ssa_exprt(member_exprt("x", symbol_exprt("y"))) with L2 = 5 would be better represented as y.x#5 than y#5.x.

@tautschnig
Copy link
Collaborator Author

tautschnig commented Jan 8, 2019

@smowton Both of these are very good points - I'd suggest to move that discussion to #2574 as here I'd really just like to fix the inconsistency bug. Indeed #2574 already includes the change to use .. and [[]].

allredj
allredj reviewed Jan 8, 2019
View reviewed changes
Copy link
Contributor

@allredj allredj left a comment

Choose a reason for hiding this comment

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

✔️
Passed Diffblue compatibility checks (cbmc commit: cedceae).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/96614633

@tautschnig tautschnig merged commit af79d4e into diffblue:develop Jan 9, 2019
@tautschnig tautschnig deleted the ssa-field-sensitivity-l1 branch January 9, 2019 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kroening kroening Awaiting requested review from kroening kroening is a code owner

@peterschrammel peterschrammel Awaiting requested review from peterschrammel peterschrammel is a code owner

@pkesseli pkesseli Awaiting requested review from pkesseli

2 more reviewers

@allredj allredj allredj left review comments

@smowton smowton smowton approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@smowton smowton

@peterschrammel peterschrammel

Labels

blocker bugfix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@tautschnig @smowton @allredj @peterschrammel