[PATCH] D89210: [Sema, CodeGen] Implement [[likely]] and [[unlikely]] in SwitchStmt

[Mark de Wever via Phabricator via cfe-commits]

Mordante marked 2 inline comments as done.
Mordante added a comment.

In D89210#2332143 <https://reviews.llvm.org/D89210#2332143>, @aaron.ballman wrote:

> LGTM aside from a documentation request, but you may want to wait a few days before committing in case Richard or John have opinions.

Thanks for the review!



================
Comment at: clang/include/clang/Basic/AttrDocs.td:1815
 
+  switch(i) {
+    [[likely]] case 1:    // This value is likely
----------------
aaron.ballman wrote:
> Can you add a second example that shows what to expect from fallthrough behavior? Perhaps something like:
> ```
> switch (i) {
> [[likely]] case 0: // This value and code path is likely
>   ...
>   [[fallthrough]];
> case 1: // No likelihood attribute, code path is likely due to fallthrough
>   break;
> case 2: // No likelihood attribute, code path is neutral
>   [[fallthrough]];
> [[unlikely]] default: // This value and code path are both unlikely
>   break;
> }
> ```
I've added this example with minor modifications. Note `case 1` is neutral since falling through has no effect.


================
Comment at: clang/lib/CodeGen/CGStmt.cpp:375
+                                     ArrayRef<const Attr *> Attrs) {
+  // clang-format off
   switch (S->getStmtClass()) {
----------------
aaron.ballman wrote:
> Mordante wrote:
> > aaron.ballman wrote:
> > > How ugly is the default clang-format formatting for this? If it's not too bad, perhaps we just re-format the switch statement as a whole?
> > It looks fine default formatted, I just thought we wanted to keep it compact. But I've no problem with keeping the code default formatted.
> I tend to prefer whatever the default formatting is just so we don't have formatting comments all over the place (but I'm fine if the unique formatting is important for code readability).
I'll keep that in mind.


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

https://reviews.llvm.org/D89210