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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
