Skip to content
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

[codemod] comment unused parameters to turn on -Wunused-parameter flag #3662

Closed
wants to merge 14 commits into from

Conversation

Dayvedde
Copy link

@Dayvedde Dayvedde commented Mar 28, 2018

This PR comments out the rest of the unused arguments which allow us to turn on the -Wunused-parameter flag. This is the second part of a codemod relating to #3557.

Test Plan:

make -j64 check
make clean && ROCKSDB_NO_FBCODE=1 make check -j64

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@Dayvedde has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@Dayvedde has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@ajkr ajkr self-requested a review March 28, 2018 18:26
@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request.

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@Dayvedde has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@ajkr has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@Dayvedde has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request.

Copy link
Contributor

@miasantreble miasantreble left a comment

Choose a reason for hiding this comment

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

Tested under freeBSD: the following files need fix:
env/env_posix.cc (Line 862)
env/io_posix.cc (Line 47, 228, 408, 465, 673, 866, 880)
util/arena.cc (Line 52, 127, 160)

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request.

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@miasantreble
Copy link
Contributor

commit ac7b1fb now builds successfully on FreeBSD

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@Dayvedde
Copy link
Author

Slowly but surely fixing all these errors...

@facebook-github-bot
Copy link
Contributor

@Dayvedde has updated the pull request. View: changes, changes since last import

@Dayvedde
Copy link
Author

Hi @miasantreble, I have fixed all of the errors!

Copy link
Contributor

@ajkr ajkr left a comment

Choose a reason for hiding this comment

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

image

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@Dayvedde has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@Dayvedde is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Apr 16, 2018
Summary:
this PR fixes a few failed contbuild:
1. ASAN memory leak in Block::NewIterator (table/block.cc:429). the proper destruction of first_level_iter_ and second_level_iter_ of two_level_iterator.cc is missing from the code after the refactoring in #3406
2. various unused param errors introduced by #3662
3. updated comment for `ForceReleaseCachedEntry` to emphasize the use of `force_erase` flag.
Closes #3718

Reviewed By: maysamyabandeh

Differential Revision: D7621192

Pulled By: miasantreble

fbshipit-source-id: 476c94264083a0730ded957c29de7807e4f5b146
maysamyabandeh pushed a commit that referenced this pull request Apr 16, 2018
Summary:
this PR fixes a few failed contbuild:
1. ASAN memory leak in Block::NewIterator (table/block.cc:429). the proper destruction of first_level_iter_ and second_level_iter_ of two_level_iterator.cc is missing from the code after the refactoring in #3406
2. various unused param errors introduced by #3662
3. updated comment for `ForceReleaseCachedEntry` to emphasize the use of `force_erase` flag.
Closes #3718

Reviewed By: maysamyabandeh

Differential Revision: D7621192

Pulled By: miasantreble

fbshipit-source-id: 476c94264083a0730ded957c29de7807e4f5b146
miasantreble added a commit that referenced this pull request Apr 20, 2018
Summary:
this PR fixes a few failed contbuild:
1. ASAN memory leak in Block::NewIterator (table/block.cc:429). the proper destruction of first_level_iter_ and second_level_iter_ of two_level_iterator.cc is missing from the code after the refactoring in #3406
2. various unused param errors introduced by #3662
3. updated comment for `ForceReleaseCachedEntry` to emphasize the use of `force_erase` flag.
Closes #3718

Reviewed By: maysamyabandeh

Differential Revision: D7621192

Pulled By: miasantreble

fbshipit-source-id: 476c94264083a0730ded957c29de7807e4f5b146
maysamyabandeh pushed a commit that referenced this pull request May 8, 2018
Summary:
this PR fixes a few failed contbuild:
1. ASAN memory leak in Block::NewIterator (table/block.cc:429). the proper destruction of first_level_iter_ and second_level_iter_ of two_level_iterator.cc is missing from the code after the refactoring in #3406
2. various unused param errors introduced by #3662
3. updated comment for `ForceReleaseCachedEntry` to emphasize the use of `force_erase` flag.
Closes #3718

Reviewed By: maysamyabandeh

Differential Revision: D7621192

Pulled By: miasantreble

fbshipit-source-id: 476c94264083a0730ded957c29de7807e4f5b146
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.

None yet

4 participants