Page MenuHomePhabricator

Add a new attribute CFNoRelease.
Needs ReviewPublic

Authored by gottesmm on Jan 28 2016, 8:37 PM.

Details

Summary

This attribute will allow users to annotate opaque c functions as not
decrementing any reference counts in the functions body or any function that is
called by the given function.

This will improve ARC optimization.

Diff Detail

Event Timeline

gottesmm updated this revision to Diff 46343.Jan 28 2016, 8:37 PM
gottesmm retitled this revision from to Add a new attribute CFNoRelease..
gottesmm updated this object.
gottesmm added a reviewer: doug.gregor.
gottesmm updated this revision to Diff 46354.Jan 28 2016, 10:39 PM

Added documentation.

aaron.ballman added a subscriber: aaron.ballman.

It's a bit strange to add an attribute that has absolutely no semantic effect whatsoever. Where is this attribute intended to be queried within the compiler? Are there additional functionality patches coming for this?

include/clang/Basic/Attr.td
540

Please, no undocumented new attributes. You should modify AttrDocs.td and include that reference here.

test/SemaObjC/attr-cf_no_release.m
31

Not that lots of test are bad, but a lot of these tests are duplicates and can be removed. For instance, really only need one test for ObjC methods, one for properties, one for params, etc. It would be good to add a test ensuring that the attribute accepts no arguments.

I think that my response via email did not hit phabriactor. So sorry for the delay.

Yes there is a forthcoming patch for CodeGen which will place an attribute on the relevant functions. The attribute will be queried in the middle end optimizer. The reason why there has been a bit of a delay is I realized I wanted to talk to a few more people about this attribute internally.

We may want to expand its use to essentially mean "no-arc", i.e. this is a c function that uses pure c-code.

There are other possibilities as well so stay tuned.

gottesmm added inline comments.Jan 31 2016, 7:12 PM
include/clang/Basic/Attr.td
540

Ok. I was just following what was done in the surrounding code. I am fine with adding something to AttrDocs.td once we pin down exactly what we want.

test/SemaObjC/attr-cf_no_release.m
31

Ok.

aaron.ballman edited edge metadata.Feb 1 2016, 7:17 AM

I think that my response via email did not hit phabriactor. So sorry for the delay.

No worries!

Yes there is a forthcoming patch for CodeGen which will place an attribute on the relevant functions. The attribute will be queried in the middle end optimizer. The reason why there has been a bit of a delay is I realized I wanted to talk to a few more people about this attribute internally.

We may want to expand its use to essentially mean "no-arc", i.e. this is a c function that uses pure c-code.

There are other possibilities as well so stay tuned.

Okay, I would recommend the future patch include that middle end component as well. It gives better context for the attribute side of things.

include/clang/Basic/Attr.td
540

Yeah, the surrounding code was grandfathered in. It really should be documented as well. ;-)