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