[PATCH] D32210: [Sema][ObjC] Add support for attribute "noescape"

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: lib/Sema/SemaDeclCXX.cpp:14024
+
+  if (OldFT->hasExtParameterInfos())
+    for (unsigned I = 0, E = OldFT->getNumParams(); I != E; ++I)
----------------
ahatanak wrote:
> rjmccall wrote:
> > I think we generally encourage the uses of braces when the nested statement is this complex, even if it is technically a single statement.
> I assume you are suggesting using braces for the if statement, not the for loop?
Yes, exactly.  This looks better, thank you.


================
Comment at: lib/Sema/SemaExpr.cpp:7251
+    if (const auto *rhproto = dyn_cast<FunctionProtoType>(rhptee))
+      rhptee = S.removeNoEscapeFromFunctionProto(lhproto, rhproto);
+
----------------
ahatanak wrote:
> rjmccall wrote:
> > I think the right place to do this is probably mergeFunctionTypes.
> When we have the following conditional operator expression, what is the type of the expression?
> 
> ```
> void func0(__attribute__((noescape)) int *a, int *b);
> void func1(int *a, __attribute__((noescape)) int *b);
> 
> c ? func0 : func1;
> ```
> 
> The standard says the type of each parameter in the composite parameter type list is the composite type of the corresponding parameters of both functions, so I guess mergeFunctionType should drop noescape from both parameters?
Yes, that sounds right.


================
Comment at: lib/Sema/SemaExpr.cpp:7199
+  const auto *ToProto = dyn_cast<FunctionProtoType>(ToType);
+  const auto *FromProto = dyn_cast<FunctionProtoType>(FromType);
+
----------------
getAs<>, please.


================
Comment at: lib/Sema/SemaExpr.cpp:7210
+  if (!FromProto->hasExtParameterInfos())
+    return FromType;
+
----------------
There's a potential for an illegal conversion here if ToProto->hasExtParameterInfos().


https://reviews.llvm.org/D32210