[PATCH] D140868: [C] Make (c ? e1 : e2) noreturn only if both e1 and e2 are noreturn

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Jan 5 06:05:23 PST 2023


aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D140868#4027635 <https://reviews.llvm.org/D140868#4027635>, @rjmccall wrote:

> In D140868#4027480 <https://reviews.llvm.org/D140868#4027480>, @MaskRay wrote:
>
>> I am not a C language lawyer :) I wonder what else should be done to move this patch forward.
>> The https://github.com/llvm/llvm-project/issues/59792 has got some traction and has been added a candidate for the next 15.x patch release https://github.com/llvm/llvm-project/milestone/18
>
> We should test that this doesn't affect functions declared using the standard C attributes.  Otherwise, I think we can go forward with this change.  Aaron, does that seem reasonable to you?  For better or worse, the different spellings of `noreturn` do have different behavior already, since the standard spellings aren't supposed to be type-affecting.  And I feel like the suggested rule here is clearly correct if we have the flexibility to set it on our own.

Yeah, I think this is the right approach. I was worried about the inconsistency with function declaration merging, but `[[noreturn]]` is required to be on the first declaration (6.7.12.6p3) and so there's no merging issue there. `_Noreturn` does not have the same requirement but neither `_Noreturn` nor `[[noreturn]]` are part of the function type anyway. So yeah, this seems like the right way to go in this case, thank you!

Please add a release note for the changes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140868/new/

https://reviews.llvm.org/D140868



More information about the cfe-commits mailing list