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

__attribute__ ((__const)) not recognized #8743

Closed
ggreif opened this issue Oct 13, 2010 · 6 comments
Closed

__attribute__ ((__const)) not recognized #8743

ggreif opened this issue Oct 13, 2010 · 6 comments
Assignees

Comments

@ggreif
Copy link
Contributor

@ggreif ggreif commented Oct 13, 2010

Bugzilla Link 8371
Resolution FIXED
Resolved on Oct 15, 2010 08:37
Version unspecified
OS All
Attachments fix and test
CC @efriedma-quic

Extended Description

We seem to have a legacy header (and no process in place to change it) which looks pretty much like this:

http://en.wikibooks.org/wiki/C++_Programming/ctype.h_header

Clang warns on the "attribute ((__const))" part:
warning: unknown attribute '__const' ignored [-Wunknown-attributes]
attribute ((__const));
^

I propose to add '__const' as an attribute, becoming synonymous to 'const'. I am about to make a patch and shall attach it here.

@ggreif
Copy link
Contributor Author

@ggreif ggreif commented Oct 13, 2010

assigned to @ggreif

@efriedma-quic
Copy link
Collaborator

@efriedma-quic efriedma-quic commented Oct 14, 2010

Hmm... per the gcc docs, "attribute ((__const))" isn't really supposed to be legal... probably an accident of keyword mapping. A patch which special-cases __const would be better, because gcc doesn't accept, for example, "attribute((__noreturn))".

@ggreif
Copy link
Contributor Author

@ggreif ggreif commented Oct 15, 2010

Thanks Eli, I'll go for the simplistic variant, it is enough for our purposes.

@ggreif
Copy link
Contributor Author

@ggreif ggreif commented Oct 15, 2010

fixed by r116570.
Contains a comment. The fix is so simple, that a test for it is (IMHO) not needed, anyway our app tests it well, so I'll notice when somebody breaks it :-)

@ggreif
Copy link
Contributor Author

@ggreif ggreif commented Oct 15, 2010

On Chandler's request, r116570 contains a test.

@ggreif
Copy link
Contributor Author

@ggreif ggreif commented Oct 15, 2010

correction: On Chandler's request, r116571 contains a test.

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 3, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants