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

macro name is a reserved identifier #4254

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

Comments

@patnyb
Copy link

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

This comment has been minimized.

Copy link
Contributor

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!

@bagder

This comment has been minimized.

Copy link
Member

commented Aug 22, 2019

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

This comment has been minimized.

Copy link
Member

commented Aug 22, 2019

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.

@patnyb

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Member

commented Aug 22, 2019

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

@bagder

This comment has been minimized.

Copy link
Member

commented Aug 22, 2019

Here's how to identify most of them:

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Member

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.

@patnyb

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Member

commented Aug 22, 2019

I never saw a PR for this

bagder added a commit that referenced this issue Aug 22, 2019
defines: avoid underscore-prefixed defines
Double-underscored or underscore plus uppoeracse letter at least.

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

Fixes #4254
bagder added a commit that referenced this issue Aug 22, 2019
defines: avoid underscore-prefixed defines
Double-underscored or underscore plus uppercase letter at least.

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

Fixes #4254
Closes #4255
@bagder

This comment has been minimized.

Copy link
Member

commented Aug 23, 2019

@patnyb does #4255 fix the issues for you?

@patnyb

This comment has been minimized.

Copy link
Author

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 in 32d64b2 Aug 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.