Skip to content

Avoid shadowing of variables (several instances) [blocks: #2310]#3415

Merged
kroening merged 14 commits intodiffblue:developfrom
tautschnig:vs-shadow-1
Dec 11, 2018
Merged

Avoid shadowing of variables (several instances) [blocks: #2310]#3415
kroening merged 14 commits intodiffblue:developfrom
tautschnig:vs-shadow-1

Conversation

@tautschnig
Copy link
Copy Markdown
Collaborator

Please review commit-by-commit.

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

Copy link
Copy Markdown
Contributor

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

Many thanks for all the cleanup - looks like a great improvement.

It's completely outside the scope of this PR of course, but it does make me wonder if we should have a play with something like clang-tidy to see if we could have a lint-like job to check for this kind of shadowing.

@chrisr-diffblue chrisr-diffblue removed their assignment Nov 14, 2018
It's another iterator already in use, just rename this instance to "entry."
Renamed the first declaration, which really we don't use as an iterator.
It is a class member, no need to pass it around as a parameter shadowing the
very same reference.
Just use a ranged for instead.
Renamed parameter to "subsumed_path".
Having just one of these as "num_symbols" is sufficient.
Both cleanup (using ranged for) and fix of shadowing.
It is never used and shadows a class member.
Rename the first declaration to "source_line"
Rename the first declaration to "lock_entry".
Use target_index instead to avoid shadowing.
We pass different sets of cycles in there, only sometimes the actual class
member itself.
This avoids shadowing.
@tautschnig
Copy link
Copy Markdown
Collaborator Author

It's completely outside the scope of this PR of course, but it does make me wonder if we should have a play with something like clang-tidy to see if we could have a lint-like job to check for this kind of shadowing.

Once #2310 is in place you won't get past CI anymore if you introduce shadowing. It's just that #2310 has quite a lot of commit left for me to put into reviewable PRs, which I'll do once I've got the current batch approved and merged.

Copy link
Copy Markdown
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: ee34d34).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/91452422

@kroening kroening merged commit 24e9b88 into diffblue:develop Dec 11, 2018
@tautschnig tautschnig deleted the vs-shadow-1 branch December 17, 2018 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants