[PATCH] D15907: Allow various function attributes to be specified on Objective-C blocks too.

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

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.


================
Comment at: include/clang/Basic/Attr.td:992
@@ -993,1 +991,3 @@
+  let Subjects = SubjectList<[ObjCMethod, HasFunctionProto, ParmVar],
+                             WarnDiag, "ExpectedFunctionMethodParameterOrBlock">;
   let Args = [VariadicUnsignedArgument<"Args">];
----------------
Since you updated ClangAttrEmitter.cpp, you shouldn't need this last argument since it can now be inferred.

================
Comment at: lib/Parse/ParseExpr.cpp:2679
@@ -2678,3 +2678,1 @@
 
-/// ParseBlockId - Parse a block-id, which roughly looks like int (int x).
-///
----------------
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.

================
Comment at: lib/Sema/SemaDeclAttr.cpp:1282
@@ -1279,1 +1281,3 @@
+                                     const AttributeList &Attr,
+                                     QualType ResultType) {
   SourceRange SR = getFunctionOrMethodResultSourceRange(D);
----------------
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.)

================
Comment at: lib/Sema/SemaDeclAttr.cpp:5654
@@ -5644,1 +5653,3 @@
+void Sema::ProcessDeclAttributes(Scope *S, Decl *D, const Declarator &PD,
+                                 QualType *OverrideRetType) {
   // Apply decl attributes from the DeclSpec if present.
----------------
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.


http://reviews.llvm.org/D15907