LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 8371 - __attribute__ ((__const)) not recognized
Summary: __attribute__ ((__const)) not recognized
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: -New Bugs (show other bugs)
Version: unspecified
Hardware: All All
: P normal
Assignee: Gabor Greif
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-13 04:42 PDT by Gabor Greif
Modified: 2010-10-15 08:37 PDT (History)
2 users (show)

See Also:
Fixed By Commit(s):


Attachments
fix and test (1.99 KB, text/plain)
2010-10-13 05:19 PDT, Gabor Greif
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gabor Greif 2010-10-13 04:42:34 PDT
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.
Comment 1 Gabor Greif 2010-10-13 05:19:52 PDT
Created attachment 5614 [details]
fix and test

A patch that strips leading '__' even when there is no trailing '__' present. And a test for it too.
Comment 2 Eli Friedman 2010-10-14 14:35:26 PDT
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))".
Comment 3 Gabor Greif 2010-10-15 03:22:42 PDT
Thanks Eli, I'll go for the simplistic variant, it is enough for our purposes.
Comment 4 Gabor Greif 2010-10-15 03:29:07 PDT
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 :-)
Comment 5 Gabor Greif 2010-10-15 03:45:56 PDT
On Chandler's request, r116570 contains a test.
Comment 6 Gabor Greif 2010-10-15 08:37:59 PDT
correction: On Chandler's request, *r116571* contains a test.