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

reserved identifier violation #1

Open
elfring opened this issue Jul 30, 2014 · 9 comments
Open

reserved identifier violation #1

elfring opened this issue Jul 30, 2014 · 9 comments

Comments

@elfring
Copy link

elfring commented Jul 30, 2014

I would like to point out that identifiers like "_PAYLOAD_" and "_SOCKET_" do not fit to the expected naming convention of the C language standard.
Would you like to adjust your selection for unique names?

@Smattr
Copy link
Contributor

Smattr commented Sep 5, 2014

Apologies for the late response. It seems none of us were actually on the notify list for this repository.

You are referring to the leading underscore, I take it. Yes, these are reserved names in C. However, in this case they are being used as header guards and not actually present in the post-processed C source. I have no objection to refactoring them to use a different naming scheme, but I also don't see an issue with their current usage.

I'm not sure what you mean by "unique names" here. How does uniqueness factor into this situation?

@elfring
Copy link
Author

elfring commented Sep 5, 2014

Include guard macros are also identifiers. How do you think about other interpretations for their relevance in standard compliance?

Would you like to reuse a name pattern that I suggested also for other software?

@Smattr
Copy link
Contributor

Smattr commented Sep 11, 2014

seL4/seL4#2 relates to headers inside the kernel. These are clearly part of the implementation, as @lsf37 has said, so underscore-prefixed identifiers are allowed in this context.

The headers referenced in this issue are a bit more of a grey area, as they are not clearly part of the runtime implementation. As I said in my previous response, I have no objection to removing the prefix underscore from these tokens.

I would object to the naming scheme you've proposed as it's unnecessarily complex, in my opinion. It is not an arbitrary, unknown namespace that the header guards are instantiated within. In the case of an application header, the application author knows enough information in advance to avoid colliding with existing symbols. In the case of a library header, prefixing the guard with the name of the library is sufficient to avoid conflicts.

@elfring
Copy link
Author

elfring commented Sep 12, 2014

Thanks for your constructive feedback.

I have got a different opinion. I would prefer to reduce the probability for name clashes even more.

@Smattr
Copy link
Contributor

Smattr commented Sep 13, 2014

In the case of generated headers, I agree that something more thorough is required, but for these static headers I think what's there is adequate. Even for generated headers though, a guard based on the absolute path of the header is guaranteed to be unique in a localised build. I don't see the need for something more opaque like a UUID. Are you happy for me to close this issue and the other linked ones?

@elfring
Copy link
Author

elfring commented Sep 13, 2014

No. - These bug reports can be closed after the affected identifiers will be improved.

Would you like to add the project name to your include guards as a prefix at least?

@Smattr
Copy link
Contributor

Smattr commented Sep 16, 2014

This repository itself doesn't need any guard prefixes, as everything in here is an application. That is to say, any author of a header here knows exactly what will and what will not be in scope at the time the header is encountered. I'm open to removing the underscore prefixes on these header guards, but this is quite low priority in relation to some of the other ongoing work.

I'm leaving this issue open for now with the outstanding work being to remove the leading underscores. After that has happened I consider this issue can be closed as resolved.

@Smattr
Copy link
Contributor

Smattr commented Aug 9, 2016

Instead of removing the reserved prefixes, these guards should be replaced with #pragma once. This could be done in the generated header as well. If you want to submit a pull request with these changes, I imagine it would be palatable.

@elfring
Copy link
Author

elfring commented Aug 9, 2016

Do you care for standard-compliance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants