Skip to content

Check symbol table mappings #3187

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

chrisr-diffblue
Copy link
Contributor

@chrisr-diffblue chrisr-diffblue commented Oct 16, 2018

This PR builds upon/depends upon #3123 (merged). It adds some basic sanity checking of the symbol table structure, namely that symbols are correctly keyed, and that base_name and module maps are in sync with the symbols.

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

@martin-cs
Copy link
Collaborator

Do you want reviews before #3123 is merged?

@chrisr-diffblue
Copy link
Contributor Author

Happy to receive reviews before #3123 - the checks added in this PR are independent of #3123, it just uses that PR for its hooks.

Copy link
Collaborator

@martin-cs martin-cs left a comment

Choose a reason for hiding this comment

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

I have only reviewed the last commit; so approval is for that, not the whole stack.

map_matches = true;

++base_map_search;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

      bool map_matches =
        std::find_if(
          symbol_base_map.begin(),
          symbol_base_map.end(),
          [&](const typename symbol_base_mapt::value_type &pair) {
            return pair.first == symbol.base_name && pair.second == symbol_key;
          }) != symbol_base_map.end();

Doesn't the while loop actually go through all the elements past the first find match?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The !map_matches condition in the while loop will terminate the loop at the first match, but I agree that your suggested use of the std library is a better way to go - I'll update, thanks :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, using equal_range is probably better....

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes equal_range would be logarithmic, unlike the above.

@chrisr-diffblue chrisr-diffblue force-pushed the check-symbol-table-mappings branch 3 times, most recently from f337e41 to a622be9 Compare October 16, 2018 13:54
Copy link
Contributor

@NlightNFotis NlightNFotis left a comment

Choose a reason for hiding this comment

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

LGTM

tautschnig added a commit that referenced this pull request Nov 9, 2018
Skeleton for well-formedness checking of goto models [blocks: #3118, #3147, #3188, #3191, #3187, #3193]
@chrisr-diffblue chrisr-diffblue force-pushed the check-symbol-table-mappings branch from a622be9 to bb11a70 Compare November 12, 2018 11:10
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: 0f18b68).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/91054169

@tautschnig tautschnig merged commit 283bdbc into diffblue:develop Nov 12, 2018
@chrisr-diffblue chrisr-diffblue deleted the check-symbol-table-mappings branch November 12, 2018 14:15
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.

7 participants