[PATCH] D112579: Allow non-variadic functions to be attributed with `__attribute__((format))`

[Aaron Ballman via Phabricator via cfe-commits]

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