Skip to content

Silence warnings about unused parameters [blocks: #2310] #2470

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
merged 2 commits into from
Nov 10, 2018

Conversation

tautschnig
Copy link
Collaborator

Their names are useful to understand the purpose of the function and/or are part
of Doxygen documentation, and thus cannot be removed.

Copy link
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.

I'm more ambivalent about these changes to silence Visual Studio compiler warnings that the other recent PR's on this topic. Would it be worth at least adding a // silence compiler warning comment with each the new (void) uses that have been added in this PR?

Copy link
Member

@peterschrammel peterschrammel left a comment

Choose a reason for hiding this comment

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

I prefer this PR to #2468.

@tautschnig
Copy link
Collaborator Author

tautschnig commented Jun 25, 2018

I prefer this PR to #2468.

Note, that they are currently complementary: where I deemed the parameter named valuable I've added a (void)parameter; - in other cases I've just removed the named (#2468). But I'm happy to consistently do (void)parameter (and add the comment that @chrisr-diffblue suggested) - just let me know!

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.

This PR failed Diffblue compatibility checks (cbmc commit: 5cabcea).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/77257469
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

  • the cbmc commit has disappeared in the mean time (e.g. in a force-push)
  • the author is not in the list of contributors (e.g. first-time contributors).

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: a139223).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/78365264

@tautschnig
Copy link
Collaborator Author

As just said in #2468: @chrisr-diffblue, @peterschrammel I have created #2568 as a proposed amendment to the coding standard.

@tautschnig
Copy link
Collaborator Author

Rebased, updated, comments added.

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.

This PR failed Diffblue compatibility checks (cbmc commit: dc2489d).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/78561014
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

  • the cbmc commit has disappeared in the mean time (e.g. in a force-push)
  • the author is not in the list of contributors (e.g. first-time contributors).

@peterschrammel peterschrammel removed their assignment Aug 4, 2018
Copy link
Member

@peterschrammel peterschrammel left a comment

Choose a reason for hiding this comment

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

@tautschnig, needs a rebase.

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'm not opposed but I think that changes to the linter / clang-format / CI are a better way of approaching this sort of thing.

@tautschnig
Copy link
Collaborator Author

I'm not opposed but I think that changes to the linter / clang-format / CI are a better way of approaching this sort of thing.

I don't think it's within the capabilities of a linter or formatter to detect those problems, but compilers would. Enabling this in CI is part of #2310, which this PR is factored out from.

@tautschnig tautschnig force-pushed the vs-void-par branch 3 times, most recently from 88917b5 to 35ba8ce Compare August 13, 2018 11:56
@tautschnig tautschnig changed the title Silence warnings about unused parameters Silence warnings about unused parameters [blocks: #2310] Nov 7, 2018
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: ab6255b).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/90533681

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.

🚫
This PR failed Diffblue compatibility checks (cbmc commit: ffb6f49).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/90581220
Status will be re-evaluated on next push.
Please contact @peterschrammel, @thk123, or @allredj for support.

Common spurious failures:

  • the cbmc commit has disappeared in the mean time (e.g. in a force-push)
  • the author is not in the list of contributors (e.g. first-time contributors).

The incompatibility may have been introduced by an earlier PR. In that case merging this
PR should be avoided unless it fixes the current incompatibility.

@kroening kroening removed their assignment Nov 7, 2018
@tautschnig tautschnig self-assigned this Nov 7, 2018
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.

⚠️
Diffblue compatibility check is currently unavailable.
Please create manual bump.
(cbmc commit: 748533a).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/90847630

Their names are useful to understand the purpose of the function and/or are part
of Doxygen documentation, and thus cannot be removed.
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: 359b3b1).
Build URL: https://travis-ci.com/diffblue/test-gen/builds/90955657

@tautschnig tautschnig merged commit 96a71f6 into diffblue:develop Nov 10, 2018
@tautschnig tautschnig deleted the vs-void-par branch November 10, 2018 20:58
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