GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Directives: remove parens to silence gcc8 warning by coneiric · Pull Request #844 · cpp-netlib/cpp-netlib

Conversation

@coneiric

@coneiric coneiric commented May 29, 2018

Copy link
Copy Markdown

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));

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@anonimal

anonimal commented May 29, 2018

Copy link
Copy Markdown

Can you also PR this to branch 0.13-release?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coneiric

coneiric commented May 29, 2018

Copy link
Copy Markdown
Author

Absolutely, will PR now.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
deanberris
deanberris reviewed May 30, 2018
View reviewed changes
Comment thread boost/network/message/directives/detail/string_directive.hpp
template <class ValueType> \
struct name##_directive { \
ValueType const&((value)); \
ValueType const& value; \

@deanberris deanberris May 30, 2018

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

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 just value to use the member variable value##_ instead?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@coneiric coneiric May 30, 2018

Copy link
Copy Markdown
Author

There was a problem hiding this comment.

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 would other.value also need to become other.value##_?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ghost

ghost commented May 30, 2018 via email

Copy link
Copy Markdown
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> .
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coneiric
coneiric force-pushed the fix-gcc8-paren-warning branch from 70fdea7 to 1848435 Compare May 31, 2018 03:35
@coneiric

coneiric commented Jun 13, 2018

Copy link
Copy Markdown
Author

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@deanberris

deanberris commented Jun 13, 2018

Copy link
Copy Markdown
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?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coneiric

coneiric commented Jun 14, 2018

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@deanberris

deanberris commented Jun 14, 2018

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@deanberris

deanberris commented Jun 14, 2018

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coneiric

coneiric commented Jun 14, 2018

Copy link
Copy Markdown
Author

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

Unfortunately, still broken... Same errors.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@deanberris

deanberris commented Jun 14, 2018

Copy link
Copy Markdown
Member

Try:

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

You should see that the submodule definitions have been updated.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coneiric

coneiric commented Jun 21, 2018

Copy link
Copy Markdown
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=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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coneiric
Directives: remove parens to silence gcc8 warning
cdf7613
@coneiric
coneiric force-pushed the fix-gcc8-paren-warning branch from 1848435 to cdf7613 Compare June 21, 2018 22:07
@deanberris

deanberris commented Jun 21, 2018

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@coneiric

coneiric commented Jun 21, 2018

Copy link
Copy Markdown
Author

I'm happy to merge now.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@deanberris
deanberris merged commit b284b14 into cpp-netlib:master Jun 21, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@deanberris deanberris deanberris left review comments

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@coneiric @anonimal @deanberris
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.