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.

Sign up for GitHub

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

Jump to bottom

macro name is a reserved identifier #4254

Closed
ghost opened this issue Aug 22, 2019 · 12 comments
Closed

macro name is a reserved identifier #4254

ghost opened this issue Aug 22, 2019 · 12 comments
Labels
build

Comments

@ghost
Copy link

ghost commented Aug 22, 2019

#148
"This is not a problem in the real world though."

Well now for me it is - is this possible to fix or do I have to manually fix it each time (curl is used in a number of projects and I don't wanna turn this flag off)?

curl: 7.62.0
compiler: embarcadero c++-builder 10.3.2/clang (5?) c++17

flags: -Weverything -Werror et al

error E2300: macro name is a reserved identifier:

include\curl\curl.h(2,9)
include\curl\curlver.h(2,9)
include\curl\system.h(2,9)
include\curl\system.h(484,13)
include\curl\system.h(488,11)
include\curl\easy.h(2,9)
include\curl\multi.h(2,9)
include\curl\urlapi.h(2,9)

https://wiki.sei.cmu.edu/confluence/display/cplusplus/DCL51-CPP.+Do+not+declare+or+define+a+reserved+identifier
"Each name that contains a double underscore __ or begins with an underscore followed by an uppercase letter is reserved to the implementation for any use."
@jzakrzewski
Copy link
Contributor

The same comment in #148 also states:
"If someone brings a patch for it then I'll merge"

So just do it once and send a pull request!

@bagder
Copy link
Member

bagder commented Aug 22, 2019 •
edited
Loading

I still stand by that comment. The defines are harmless and won't cause anyone any troubles, but if you go ahead and fix them and I'll accept the PR.

@bagder bagder added the build label Aug 22, 2019
@bagder
Copy link
Member

bagder commented Aug 22, 2019 •
edited
Loading

BTW, clang's -Weverything option warns on totally fine things as well in some misguided attempt to be "helpful" which makes the option less useful to use when building curl and I'll advice against it.

Things like that it pads structs or finds "unknown command tag name" (email addresses) in comments.

@ghost
Copy link
Author

ghost commented Aug 22, 2019

The defines are harmless and won't cause anyone any troubles

Obviously they are causing me trouble so "almost anyone" ...

.. when building curl ...

This is happening when compiling projects including curl ...

@bagder
Copy link
Member

bagder commented Aug 22, 2019

They trigger warnings with that clang option enabled, does it cause any other (real) harm?

@bagder
Copy link
Member

bagder commented Aug 22, 2019

Here's how to identify most of them:

find . -name "*.[ch]" | xargs grep '#define *_'

@ghost
Copy link
Author

ghost commented Aug 22, 2019

They trigger warnings with that clang option enabled, does it cause any other (real) harm?

[In my case warnings=errors] Apart from being non standard compliant, of course not.

I'll try to upgrade to the latest version of curl next week and fix a patch.

@bagder
Copy link
Member

bagder commented Aug 22, 2019

The combination of those flags causes this build problem but without -Weverything, -Werror is fine and we use it as a standard in our builds.

@ghost
Copy link
Author

ghost commented Aug 22, 2019

The same comment in #148 also states:
"If someone brings a patch for it then I'll merge"

So just do it once and send a pull request!

Well elfring did it seems.

@bagder
Copy link
Member

bagder commented Aug 22, 2019

I never saw a PR for this

@bagder bagder mentioned this issue Aug 22, 2019
Closed
bagder added a commit that referenced this issue Aug 22, 2019
@bagder
defines: avoid underscore-prefixed defines

Verified

This commit was signed with the committer’s verified signature.
bagder Daniel Stenberg
GPG key ID: 5CC908FDB71E12C2
Verified
Learn about vigilant mode
e7e564c 
Double-underscored or underscore plus uppercase letter at least.

... as they're claimed to be reserved.

Fixes #4254
Closes #4255
@bagder
Copy link
Member

bagder commented Aug 23, 2019

@patnyb does #4255 fix the issues for you?

@ghost
Copy link
Author

ghost commented Aug 23, 2019

@patnyb does #4255 fix the issues for you?

Yes - all curl-related warnings are gone - thanks alot! :)

@bagder bagder closed this as completed in 32d64b2 Aug 23, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 21, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
build
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

defines: avoid underscore-prefixed defines curl/curl
2 participants
@bagder @jzakrzewski