[PATCH] Implement the flatten attribute.

Aaron Ballman aaron at aaronballman.com
Tue May 20 06:02:12 PDT 2014


> Index: include/clang/Basic/AttrDocs.td
> ===================================================================
> --- include/clang/Basic/AttrDocs.td
> +++ include/clang/Basic/AttrDocs.td
> @@ -868,6 +868,14 @@
>    }];
>  }
>
> +def FlattenDocs : Documentation {
> +  let Category = DocCatFunction;
> +  let Content = [{
> +The ``flatten`` attribute causes functions called by the function with the
> +attribute to be inlined if possible.

Aren't functions already inlined if possible, regardless of this
attribute? Also, it may be a bit more clear to say "calls within the
attributed function" instead of "functions called by the function."

> +  }];
> +}

Other than that, LGTM!

~Aaron

On Mon, May 19, 2014 at 6:21 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> On Mon, May 19, 2014 at 02:44:40PM -0400, Aaron Ballman wrote:
>> > Index: include/clang/Basic/Attr.td
>> > ===================================================================
>> > --- include/clang/Basic/Attr.td
>> > +++ include/clang/Basic/Attr.td
>> > @@ -668,6 +668,12 @@
>> >    let Documentation = [Undocumented];
>> >  }
>> >
>> > +def Flatten : InheritableAttr {
>> > +  let Spellings = [GNU<"flatten">];
>>
>> This should be spelled with a GCC spelling instead of GNU.
>
> Done.
>
>> > +  let Subjects = SubjectList<[Function]>;
>>
>> What about member functions? Templates? Obj-C methods?
>
> First two yes, last not sure.
>
>> > +  let Documentation = [Undocumented];
>>
>> Please document this attribute prior to committing.
>
> Done.
>
>> Second if can be hoisted into the first if. That being said, I prefer
>> the way Alp proposed to implement it as it is a bit more concise and
>> doesn't require a signature change where the caller is required.
>
> I decided to change it to the way Alp proposed.
>
>> Missing test cases for semantics of the attribute (that it applies to
>> the subjects you expect, takes no arguments, etc).
>
> Added.
>
> Thanks,
> --
> Peter



More information about the cfe-commits mailing list