Skip to content

Resolve symbol types in symbol table #2

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

Conversation

danpoe
Copy link

@danpoe danpoe commented Jun 21, 2018

This resolves the type symbols to the pointed-to types in the symbol table before creating the goto program, when reading a json goto program file.

@danpoe danpoe requested a review from chrisr-diffblue as a code owner June 21, 2018 14:25
Copy link
Owner

@chrisr-diffblue chrisr-diffblue left a comment

Choose a reason for hiding this comment

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

Also, we should figure out how to synchronize this branch with @smowton's original branch....

irept &irep,
const namespacet &ns)
{
if(irep.id() == ID_symbol)
Copy link
Owner

Choose a reason for hiding this comment

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

Can't this whole if block be replaced with a single call to ns.follow_symbol(irep) ?

Copy link
Author

Choose a reason for hiding this comment

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

The follow_symbol() method follows both type symbols and non-type symbols, but we only want to replace type symbols. So we first do the check if(symbol.is_type) and only do the follow in this case.

Copy link
Owner

Choose a reason for hiding this comment

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

Ah, right - got it... A couple of observations:

  1. This should probably be renamed to follow_type_symbols to match the naming of follow_symbols
  2. Should either this follow_type_symbols function be moved into namespace.cpp as a utility function there, or should ns.follow_symbol() grow an extra argument (with a default value) that specifies whether to follow non-type symbols or not, and then just use that where we need it instead?

Copy link
Author

Choose a reason for hiding this comment

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

Ah yes. I think I'll move the new follow_type_symbols function to namespace.cpp.

@danpoe danpoe force-pushed the feature/resolve-symbol-types-in-symbol-table branch from af2e8bd to e262b18 Compare June 26, 2018 15:52
@danpoe danpoe force-pushed the feature/resolve-symbol-types-in-symbol-table branch from e262b18 to 95af4f9 Compare June 26, 2018 17:20
@chrisr-diffblue chrisr-diffblue merged commit 5d0c656 into chrisr-diffblue:external_parser_support_rebase Jul 5, 2018
chrisr-diffblue pushed a commit that referenced this pull request Aug 17, 2018
chrisr-diffblue pushed a commit that referenced this pull request Mar 28, 2019
We should keep the struct tag type, not follow it and get the struct type
@danpoe danpoe deleted the feature/resolve-symbol-types-in-symbol-table branch June 2, 2020 17:19
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