[cfe-commits] [PATCH] "Prefix" FunctionType attributes on C++ constructors, destructors and conversion operators are ignored.

[Douglas Gregor]

On Sep 15, 2010, at 11:26 AM, Benoit Belley wrote:

> Hi everyone,
> 
> The attached patch fixes a problem where "prefix" FunctionType attributes on C++ constructors, destructors and conversion operators are being ignored.
> 
> My first change is in TypeXML.def. It augments the -ast-print-xml functionality to print the noreturn and regparm FunctionType attributes. This functionality allows me to demonstrate the issue.
> 
> If we compile the following code, making use of "postfix" FunctionType attributes:
> 
> ————————————————————————————————————
> void f() __attribute__((noreturn));
> 
> struct A
> {
>    A() __attribute__((noreturn)) ;
>    ~A() __attribute__((noreturn)) ;
>    operator int() __attribute__((noreturn)) ;
>    void foo() __attribute__((noreturn)) ;
> };
> ————————————————————————————————————
> 
> $ clang++ -cc1 -ast-print-xml foo.cpp
> 
> and looked at the foo.xml output, we will see that the FuntionType of all 5 declared functions have the noreturn attribute set.
> 
> 
> But, if we compile the following code, making use of "prefix" FunctionType attributes:
> 
> ————————————————————————————————————
> __attribute__((noreturn)) void f();
> 
> struct A
> {
>    __attribute__((noreturn)) A();
>    __attribute__((noreturn)) ~A();
>    __attribute__((noreturn)) operator int();
>    __attribute__((noreturn)) void foo();
> };
> 
> ————————————————————————————————————
> 
> $ clang++ -cc1 -ast-print-xml foo.cpp
> 
> and looked at the foo.xml output, we will see that only the FuntionType of the f() and foo() function have the noreturn attribute set. The constructor, destructor and conversion operator are all missing the noreturn function attribute.
> 
> The problem also occurs with the other FunctionType attributes (calling conventions, register parameters, etc...).
> 
> The solution that I have found is to move the code scanning for DeclSpec attributes from the ConvertDeclSpecToType() function (which is only invoked on non-special functions) to the GetTypeForDeclarator() function (which is invoked for all types of functions).
> 
> I have verified that all Clang unit tests pass after my changes.

That seems reasonable. Please update the comment on ConvertDeclSpecToType() to specify that it does *not* add the DeclSpec attributes to the type, just in case we get another caller at some point.

> BTW, what would the appropriate approach for creating a unit test for testing this functionality ?

I suggest adding a CodeGenCXX test, which emits LLVM IR. Then, use FileCheck to make sure that attribute made it all the way down to IR.

With comment + test case, patch looks good!

	- Doug