This is an archive of the discontinued LLVM Phabricator instance.

Allow various function attributes to be specified on Objective-C blocks too.
Needs ReviewPublic

Authored by comex on Jan 5 2016, 3:08 PM.

Details

Summary

This mostly "just works" by adding Block to the subject list, but there is an
issue with warnings in attribute handlers tied to the return type, which for
blocks can be inferred. My solution to this is a bit ugly but seems to do the
right thing.

The list: always_inline, noinline, cold, hot, minsize, malloc,
disable_tail_calls, noalias, noduplicate, nonnull, returns_nonnull, optnone.

nonnull already partially worked on blocks, but fix applying it to parameters
on them; also, improve the error message and add additional tests.

Most of these attributes only make sense when the target of a function call is
known; since blocks are always called indirectly via pointers, these will only
work if the optimizer is able to replace the indirect calls with direct calls
(ergo not at all at -O0). However, this can still be useful in practice.

For now, all of them only apply to the block implementation function itself, as
opposed to the copy and dispose helpers. For those it might make sense to
propagate always_inline in particular, or perhaps to just add some explicit
syntax for putting attributes on them, but it's not essential.

Incidentally, for some of these attributes and some not included, such as
returns_nonnull, printf, warn_unused_result, etc., it would be somewhat useful
and more principled to allow them as part of function types rather than just
functions themselves, for the sake of both standard function pointer calls and
blocks. Currently only a handful of attributes can be used on types: noreturn,
ns_returns_retained, regparm, and calling convention. However, that would be a
larger change and orthogonal to this patch.

Diff Detail

Event Timeline

comex updated this revision to Diff 44065.Jan 5 2016, 3:08 PM
comex retitled this revision from to Allow various function attributes to be specified on Objective-C blocks too..
comex updated this object.
comex added a subscriber: cfe-commits.

Ping, and adding potential reviewers like I was supposed to do in the first place.

aaron.ballman edited edge metadata.Jan 14 2016, 6:34 AM

At a high level, is there a reason why these should be supported on block, but not on ObjC method declarations? It smells like these may be useful there as well, which could also simplify the patch slightly.

include/clang/Basic/Attr.td
992

Since you updated ClangAttrEmitter.cpp, you shouldn't need this last argument since it can now be inferred.

lib/Parse/ParseExpr.cpp
2679

This (and its related changes) look good to me, but should be in a separate patch since they are orthogonal to the rest of the changes in this patch.

lib/Sema/SemaDeclAttr.cpp
1282

Please do not add any parameters to these function signatures. We try to keep them uniform in the hopes that maybe someday we can refactor this code to do dispatch more easily. (It's fine to add an extra level of indirection through a helper function that then has the additional parameter.)

5654

This doesn't seem like the correct way to solve the problem -- everyone who calls ProcessDeclAttributes should not have to understand what an overridden return type is for objective-c blocks. I'm not certain of a better way to solve it right now, but I would rather see the changes for this one split out into its own patch so we can make progress on the other improvements.