Skip to content

cpp_scopet::lookup methods now return an id set #3063

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Sep 29, 2018
Merged

cpp_scopet::lookup methods now return an id set #3063

merged 1 commit into from
Sep 29, 2018

Conversation

kroening
Copy link
Member

  • Each commit message has a non-empty body, explaining why the change was made.
  • My contribution is formatted in line with CODING_STANDARD.md.
  • 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.

allredj
allredj reviewed Sep 29, 2018
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: bfe7eed).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/86364498

tautschnig
tautschnig approved these changes Sep 29, 2018
View reviewed changes
Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

I may be wrong on some of the below, but it looks like a lot of const-ness could be added.

src/cpp/cpp_instantiate_template.cpp Outdated
@@ -335,8 +335,8 @@ const symbolt &cpp_typecheckt::instantiate_template(
{
cpp_scopet &scope=cpp_scopes.get_scope(subscope_name);

cpp_scopet::id_sett id_set;
scope.lookup(template_symbol.base_name, cpp_scopet::SCOPE_ONLY, id_set);
cpp_scopet::id_sett id_set =
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

src/cpp/cpp_declarator_converter.cpp Outdated

cpp_typecheck.cpp_scopes.current_scope().lookup(
base_name, cpp_scopet::SCOPE_ONLY, id_set);
cpp_scopest::id_sett id_set =
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

src/cpp/cpp_declarator_converter.cpp Outdated

scope->lookup_identifier(
symbol.name, cpp_idt::id_classt::TEMPLATE_PARAMETER, id_set);
cpp_scopet::id_sett id_set = scope->lookup_identifier(
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

src/cpp/cpp_scope.cpp Outdated
bool cpp_scopet::contains(const irep_idt &base_name)
{
id_sett id_set;
lookup(base_name, SCOPE_ONLY, id_set);
const id_sett id_set = lookup(base_name, SCOPE_ONLY);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe this should just be rewritten as return !lookup(base_name, SCOPE_ONLY).empty();

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

src/cpp/cpp_typecheck_compound_type.cpp Outdated
@@ -105,8 +105,8 @@ cpp_scopet &cpp_typecheckt::tag_scope(
// Check if we have it already. If so, take it.

// we should only look for tags, but we don't
cpp_scopet::id_sett id_set;
cpp_scopes.current_scope().lookup(base_name, cpp_scopet::RECURSIVE, id_set);
cpp_scopet::id_sett id_set =
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

src/cpp/cpp_typecheck_template.cpp Outdated
cpp_scopet::SCOPE_ONLY,
cpp_scopet::TEMPLATE,
id_set);
cpp_scopet::id_sett id_set=
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

src/cpp/cpp_typecheck_template.cpp Outdated
cpp_scopet::id_sett id_set;

cpp_scopes.current_scope().lookup(
cpp_scopet::id_sett id_set = cpp_scopes.current_scope().lookup(
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

src/cpp/cpp_typecheck_template.cpp Outdated
cpp_scopest::id_sett id_set;
cpp_scopes.current_scope().lookup(
base_name, cpp_scopet::SCOPE_ONLY, cpp_idt::id_classt::TEMPLATE, id_set);
cpp_scopest::id_sett id_set = cpp_scopes.current_scope().lookup(
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

src/cpp/cpp_typecheck_template.cpp Outdated
cpp_scopest::id_sett id_set;
cpp_scopes.current_scope().lookup(
base_name, cpp_scopet::SCOPE_ONLY, id_set);
cpp_scopest::id_sett id_set =
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

src/cpp/cpp_typecheck_using.cpp Outdated

cpp_scopes.current_scope().lookup(
base_name, qualified?cpp_scopet::QUALIFIED:cpp_scopet::RECURSIVE, id_set);
cpp_scopest::id_sett id_set = cpp_scopes.current_scope().lookup(
Copy link
Collaborator

Choose a reason for hiding this comment

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

const

allredj
allredj reviewed Sep 29, 2018
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: 89b2bb3).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/86369058

@kroening kroening merged commit 63bf323 into develop Sep 29, 2018
@kroening kroening deleted the cpp_scopet_lookup branch September 29, 2018 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@allredj allredj allredj left review comments

@tautschnig tautschnig tautschnig approved these changes

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

Assignees
No one assigned
Labels
C++ Front End cleanup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@kroening @tautschnig @allredj