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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
