aaron.ballman added a comment. In D112579#3625195 <https://reviews.llvm.org/D112579#3625195>, @fcloutier wrote:
> Thanks, Aaron. I wasn't sure how to follow up given how long it had been > since the review started. I understand that we're all busy (which explains > the week delay on my part here as well). No worries, pinging the review like you did is a good way to try to get it more attention, though it sometimes takes a few tries depending on the review. > I've addressed all of your comments except the one on this bit > <https://reviews.llvm.org/D112579#inline-1231865>: > > if (const FunctionType *FnTy = D->getFunctionType()) > IsVariadic = cast<FunctionProtoType>(FnTy)->isVariadic(); > > The proposed change isn't identical because `D->getFunctionType()` can return > nullptr (for instance, if `D` is a `BlockDecl`). However, in the case `FnTy` > isn't nullptr, then it is guaranteed to be a `FunctionProtoType` as the > attribute is rejected on functions without a prototype. The suggestion I had was slightly different: if (const auto *FnTy = D->getType()->getAs<FunctionProtoType>()) IsVariadic = FnTy->isVariadic(); It's getting as a prototyped function, and only if that succeeds do we check whether it's variadic. I think that is equivalent to what you have now, but is more clearly expressed. WDYT? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112579/new/ https://reviews.llvm.org/D112579 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
