[PATCH] __format__ attribute with function pointers

Hi,

Here's a patch to fix the handling of the __format__ attribute with function
pointers.

Regards,
Nuno

clang_format_attr_fun_ptrs.txt (2.17 KB)

May I shamelessly ping this? :slight_smile:
(the patch is at http://lists.cs.uiuc.edu/pipermail/cfe-dev/2008-March/001226.html)

Regards,
Nuno

Hi Nuno,

Thanks for the ping, and sorry for the delay!

The patch looks pretty good to me, but I do have one concern/question. I noticed that you are relying on the function having a type of FunctionTypeProto. This seems to rule out attaching a format attribute to a K&R style function prototype (where the parameters are omitted). I'm not certain what GCC allows. Certainly when the type of the function is FunctionTypeProto you should perform extra checking to see if the number of arguments line up, etc., but if GCC supports K&R style function prototypes to have the format attribute then you should also support when the type is FunctionTypeNoProto (and just do less checking). If GCC does indeed disallow K&R style prototypes to have the format attribute, it would be great if you could document this assumption with a comment. Otherwise, we should probably handle the case where the type is FunctionTypeNoProto.

Ted

Hi,

The patch looks pretty good to me, but I do have one concern/ question. I noticed that you are relying on the function having a type of FunctionTypeProto. This seems to rule out attaching a format attribute to a K&R style function prototype (where the parameters are omitted). I'm not certain what GCC allows. Certainly when the type of the function is FunctionTypeProto you should perform extra checking to see if the number of arguments line up, etc., but if GCC supports K&R style function prototypes to have the format attribute then you should also support when the type is FunctionTypeNoProto (and just do less checking). If GCC does indeed disallow K&R style prototypes to have the format attribute, it would be great if you could document this assumption with a comment. Otherwise, we should probably handle the case where the type is FunctionTypeNoProto.

Thanks for reviewing the patch! (the hurry was more because this patch fixes hundreds of bogus warnings in the PHP source).
I've checked gcc 4.1 and it completely ignores the format attribute in K&R declarations. Well and who uses old stuff together with new __attribute__? :slight_smile:

Thanks,
Nuno

Hi,

The patch looks pretty good to me, but I do have one concern/ question. I noticed that you are relying on the function having a type of FunctionTypeProto. This seems to rule out attaching a format attribute to a K&R style function prototype (where the parameters are omitted). I'm not certain what GCC allows. Certainly when the type of the function is FunctionTypeProto you should perform extra checking to see if the number of arguments line up, etc., but if GCC supports K&R style function prototypes to have the format attribute then you should also support when the type is FunctionTypeNoProto (and just do less checking). If GCC does indeed disallow K&R style prototypes to have the format attribute, it would be great if you could document this assumption with a comment. Otherwise, we should probably handle the case where the type is FunctionTypeNoProto.

Thanks for reviewing the patch! (the hurry was more because this patch fixes hundreds of bogus warnings in the PHP source).
I've checked gcc 4.1 and it completely ignores the format attribute in K&R declarations.

Great! If you could regenerate the patch that is synced with the latest revision and send it my way that would be great; I'll then go and apply it. If you could also add a comment about the K&R stuff (one line comment: GCC ignores attributes on ...) that would also be great.

Well and who uses old stuff together with new __attribute__? :slight_smile:

The world of C and C++ is full of mongrels. I've seen stranger combinations.