Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Directives: remove parens to silence gcc8 warning#844

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged

Conversation

@coneiric
Copy link

Fixes-Wparentheses warning from gcc8 builds.

Example build output:

cpp-netlib/boost/network/protocol/http/message/directives/status_message.hpp:17:1: note: in expansion of macro 'BOOST_NETWORK_STRING_DIRECTIVE'                                                                                                                 BOOST_NETWORK_STRING_DIRECTIVE(status_message, status_message_,                                                                             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                                                                                                             cpp-netlib/boost/network/message/directives/detail/string_directive.hpp:37:21: warning: unnecessary parentheses in declaration of 'uri_' [-Wparentheses]                                                                                                            ValueType const&((value));

@anonimal
Copy link

Can you also PR this to branch 0.13-release?

@coneiric
Copy link
Author

Absolutely, will PR now.

template<classValueType> \
structname##_directive { \
ValueTypeconst&((value)); \
ValueTypeconst&value; \
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this change. I think, though, a better way to go about this might be to use a different name for the member variable, and use the token concatenation to do this properly. Say, something like:

ValueType const& value##_;explicit name##_directive(ValueType const& v) : value##_(v) {}

Then also change all the occurences of justvalue to use the member variablevalue##_ instead?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for this change. I think, though, a better way to go about this might be to use a different name for the member variable, and use the token concatenation to do this properly.

You're welcome. I can make the changes to the member variable using token concatenation. I'm not very familiar with macro black magic, so wanted to make the most conservative changes possible.

Just so I understand you correctly, the changes would be:

... : value##_(value_) {}... : value##_(other.value) {}

in addition to your example above? Or wouldother.value also need to becomeother.value##_?

@ghost
Copy link

ghost commentedMay 30, 2018 via email

On Thu, 31 May 2018 at 3:10 am, oneiric ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In boost/network/message/directives/detail/string_directive.hpp <#844 (comment)>: > @@ -34,7 +34,7 @@ #define BOOST_NETWORK_STRING_DIRECTIVE(name, value, body, pod_body) \ template <class ValueType> \ struct name##_directive { \ - ValueType const&((value)); \ + ValueType const& value; \ Thanks for this change. I think, though, a better way to go about this might be to use a different name for the member variable, and use the token concatenation to do this properly. You're welcome. I can make the changes to the member variable using token concatenation. I'm not very familiar with macro black magic, so wanted to make the most conservative changes possible. Just so I understand you correctly, the changes would be: ... : value##_(value_) {} ... : value##_(other.value) {} in addition to your example above? Or would other.value also need to become other.value##_?
No worries, this is not easy stuff. 😊I suggest changing the name of the function argument to something visuallydifferent from `value` like `v` then change the member name to have asuffix. So yes, `other.value` becomes `other.value##_`.—
…
You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#844 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AADr6S_zlDGfbn13j3jJ0e3piMXMlPZpks5t3tJ1gaJpZM4UR-eX> .

@coneiricconeiricforce-pushed thefix-gcc8-paren-warning branch from70fdea7 to1848435CompareMay 31, 2018 03:35
@coneiric
Copy link
Author

I've made the changes to get a working build in the0.13-release version of this pull request.

I would like to add the same changes to this PR, but there appears to be a missing commit for the_ext/breathe submodule:

error: Server does not allow request for unadvertised object 853385ef4f0c3dd126887750e20d5f7456065998Fetched in submodule path 'libs/network/doc/_ext/breathe', but it did not contain 853385ef4f0c3dd126887750e20d5f7456065998. Direct fetching of that commit failed.

Rebasing on the current master did not work either, as it also fails to build:

cpp-netlib/deps/uri/src/boost/mpl/assert.hpp:188:21: error: unnecessary parentheses in declaration of 'assert_arg' [-Werror=p$rentheses]                                                                                                                                   failed ************ (Pred::************

Several errors like the one above are issued. Please advise, and thank you for your time.

@deanberris
Copy link
Member

I've just merged a couple of PRs that were meant to address this. You're probably going to have to update the submodules after your rebase:

https://git-scm.com/docs/git-submodule#git-submodule-update--init--remote-N--no-fetch--no-recommend-shallow-f--force--checkout--rebase--merge--referenceltrepositorygt--depthltdepthgt--recursive--jobsltngt--ltpathgt82308203

Does that work?

@coneiric
Copy link
Author

You're probably going to have to update the submodules

I've updated my submodules before and after your post, no change. Master does not build with the error messages posted above.

While we are working on this issue, would you please merge the change for the0.13-release branch, since this issue does not exist there?

@deanberris
Copy link
Member

Okay, I thought#847 did that for the master branch but it actually did it for the 0.13-release branch -- my bad. Yes, I'll cherry-pick that and attempt to push.

@deanberris
Copy link
Member

Alright, please rebase past604d611 on master. Let me know if that works.

@coneiric
Copy link
Author

Alright, please rebase past604d611 on master. Let me know if that works.

Unfortunately, still broken... Same errors.

@deanberris
Copy link
Member

Try:

# after rebasing your branch onto mastergit submodule update --remotegit submodule update

You should see that the submodule definitions have been updated.

@coneiric
Copy link
Author

I understand that you updated the submodules. However, the master build is still borked:

In file included from cpp-netlib/deps/uri/src/boost/mpl/aux_/na_assert.hpp:23,                 from cpp-netlib/deps/uri/src/boost/mpl/arg.hpp:25,                                                                            from cpp-netlib/deps/uri/src/boost/mpl/placeholders.hpp:24,                 from cpp-netlib/deps/uri/src/boost/iterator/iterator_categories.hpp:17,                                                      from cpp-netlib/deps/uri/src/boost/iterator/iterator_facade.hpp:14,                 from cpp-netlib/deps/uri/src/boost/range/iterator_range_core.hpp:27,                 from cpp-netlib/deps/uri/src/detail/../boost/algorithm/string/find.hpp:16,                 from cpp-netlib/deps/uri/src/detail/uri_resolve.cpp:16:cpp-netlib/deps/uri/src/boost/mpl/assert.hpp:188:21: error: unnecessary parentheses in declaration of 'assert_arg' [-Werror=parentheses]                                                                                                                                   failed ************ (Pred::************                     ^                                            cpp-netlib/deps/uri/src/boost/mpl/assert.hpp:193:21: error: unnecessary parentheses in declaration of 'assert_not_arg' [-Werror=parentheses]                                                   failed ************ (network_boost::mpl::not_<Pred>::************                     ^                                         In file included from cpp-netlib/deps/uri/src/boost/mpl/aux_/na_assert.hpp:23,                                                                 from cpp-netlib/deps/uri/src/boost/mpl/arg.hpp:25,                 from cpp-netlib/deps/uri/src/boost/mpl/placeholders.hpp:24,                                                                   from cpp-netlib/deps/uri/src/boost/iterator/iterator_categories.hpp:17,                 from cpp-netlib/deps/uri/src/boost/iterator/iterator_adaptor.hpp:14,                                                         from cpp-netlib/deps/uri/src/boost/iterator/transform_iterator.hpp:12,                 from cpp-netlib/deps/uri/src/boost/algorithm/string/iter_find.hpp:17,                 from cpp-netlib/deps/uri/src/detail/../boost/algorithm/string/split.hpp:16,                 from cpp-netlib/deps/uri/src/detail/uri_normalize.cpp:14:cpp-netlib/deps/uri/src/boost/mpl/assert.hpp:188:21: error: unnecessary parentheses in declaration of 'assert_arg' [-Werror=parentheses]                    failed ************ (Pred::************                     ^cpp-netlib/deps/uri/src/boost/mpl/assert.hpp:193:21: error: unnecessary parentheses in declaration of 'assert_not_arg' [-Werror=parentheses] failed ************ (network_boost::mpl::not_<Pred>::************

This occurs with both versions of the submodules.

@coneiricconeiricforce-pushed thefix-gcc8-paren-warning branch from1848435 tocdf7613CompareJune 21, 2018 22:07
@deanberris
Copy link
Member

LGTM

That's now an issue with the URI library, probably want to either get@glynos to update the implementation of Boost things in there or consider turning off that warning in the URI builds. I'm happy to merge now.

@coneiric
Copy link
Author

I'm happy to merge now.

Thank you@deanberris. Hopefully,@glynos will be able to help with the URI issue.

@deanberrisdeanberris merged commitb284b14 intocpp-netlib:masterJun 21, 2018
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@deanberrisdeanberrisdeanberris left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@coneiric@anonimal@deanberris
[8]ページ先頭
©2009-2025 Movatter.jp