Directives: remove parens to silence gcc8 warning #844

coneiric · 2018-05-29T18:14:19Z

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 'BOO
ST_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 · 2018-05-29T22:55:45Z

Can you also PR this to branch 0.13-release?

coneiric · 2018-05-29T23:18:30Z

Absolutely, will PR now.

deanberris · 2018-05-30T03:52:05Z

boost/network/message/directives/detail/string_directive.hpp

@@ -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. Say, something like:

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

Then also change all the occurences of just value to use the member variable value##_ instead?

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##_?

ghost · 2018-05-30T22:09:37Z

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 visually different from `value` like `v` then change the member name to have a suffix. 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> .

coneiric · 2018-06-13T20:15:20Z

I've made the changes to get a working build in the 0.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 853385ef4f0c3dd126887750e20d5f7456065998
Fetched 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 · 2018-06-13T22:59:30Z

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 · 2018-06-14T03:11:31Z

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 the 0.13-release branch, since this issue does not exist there?

deanberris · 2018-06-14T03:54:22Z

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 · 2018-06-14T04:21:56Z

Alright, please rebase past 604d611 on master. Let me know if that works.

coneiric · 2018-06-14T18:44:59Z

Alright, please rebase past 604d611 on master. Let me know if that works.

Unfortunately, still broken... Same errors.

deanberris · 2018-06-14T22:56:51Z

Try:

# after rebasing your branch onto master
git submodule update --remote
git submodule update

You should see that the submodule definitions have been updated.

coneiric · 2018-06-21T22:00:05Z

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=pa
rentheses]                                                                                                                                   failed ************ (Pred::************
                     ^                                            
cpp-netlib/deps/uri/src/boost/mpl/assert.hpp:193:21: error: unnecessary parentheses in declaration of 'assert_not_arg' [-Werro
r=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=pa
rentheses]                   
 failed ************ (Pred::************
                     ^
cpp-netlib/deps/uri/src/boost/mpl/assert.hpp:193:21: error: unnecessary parentheses in declaration of 'assert_not_arg' [-Werro
r=parentheses]
 failed ************ (network_boost::mpl::not_<Pred>::************

This occurs with both versions of the submodules.

deanberris · 2018-06-21T22:53:49Z

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 · 2018-06-21T22:56:03Z

I'm happy to merge now.

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

deanberris reviewed May 30, 2018

View reviewed changes

coneiric force-pushed the fix-gcc8-paren-warning branch from 70fdea7 to 1848435 Compare May 31, 2018 03:35

Directives: remove parens to silence gcc8 warning

cdf7613

coneiric force-pushed the fix-gcc8-paren-warning branch from 1848435 to cdf7613 Compare June 21, 2018 22:07

deanberris merged commit b284b14 into cpp-netlib:master Jun 21, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Directives: remove parens to silence gcc8 warning #844

Directives: remove parens to silence gcc8 warning #844

Uh oh!

coneiric commented May 29, 2018

Uh oh!

anonimal commented May 29, 2018

Uh oh!

coneiric commented May 29, 2018

Uh oh!

deanberris May 30, 2018

Uh oh!

coneiric May 30, 2018

Uh oh!

ghost commented May 30, 2018 via email

Uh oh!

coneiric commented Jun 13, 2018

Uh oh!

deanberris commented Jun 13, 2018

Uh oh!

coneiric commented Jun 14, 2018

Uh oh!

deanberris commented Jun 14, 2018

Uh oh!

deanberris commented Jun 14, 2018

Uh oh!

coneiric commented Jun 14, 2018

Uh oh!

deanberris commented Jun 14, 2018

Uh oh!

coneiric commented Jun 21, 2018

Uh oh!

deanberris commented Jun 21, 2018

Uh oh!

coneiric commented Jun 21, 2018

Uh oh!

Uh oh!

Directives: remove parens to silence gcc8 warning #844

Directives: remove parens to silence gcc8 warning #844

Uh oh!

Conversation

coneiric commented May 29, 2018

Uh oh!

anonimal commented May 29, 2018

Uh oh!

coneiric commented May 29, 2018

Uh oh!

deanberris May 30, 2018

Choose a reason for hiding this comment

Uh oh!

coneiric May 30, 2018

Choose a reason for hiding this comment

Uh oh!

ghost commented May 30, 2018 via email

Uh oh!

coneiric commented Jun 13, 2018

Uh oh!

deanberris commented Jun 13, 2018

Uh oh!

coneiric commented Jun 14, 2018

Uh oh!

deanberris commented Jun 14, 2018

Uh oh!

deanberris commented Jun 14, 2018

Uh oh!

coneiric commented Jun 14, 2018

Uh oh!

deanberris commented Jun 14, 2018

Uh oh!

coneiric commented Jun 21, 2018

Uh oh!

deanberris commented Jun 21, 2018

Uh oh!

coneiric commented Jun 21, 2018

Uh oh!

Uh oh!