[libc++][regex] add _LIBCPP_FALLTHROUGH to suppress fallthrough warning

[vegerot]

Summary:
The diff #97926 is stacked on top of this patch because this file reports an error when enabling -Wimplicit-fallthrough in -Wextra.

Test plan:

$ time (mkdir build_runtimes && cd build_runtimes && set -x && CC=../build/bin/clang CXX=../build/bin/clang++ cmake -G Ninja ../runtimes -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_RUNTIMES='libcxx;libcxxabi;libunwind'  && ninja && bin/llvm-lit -sv ../libcxx/test/std/re )

note: whether I put a break; or fallthrough, the tests pass anyways which is sus.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-libcxx

Author: Max Coplan (vegerot)

Changes

The diff #97926 is stacked on top of this patch because this file reports an error when enabling -Wimplicit-fallthrough in -Wextra

---

Full diff: https://github.com/llvm/llvm-project/pull/100821.diff

1 Files Affected:

• (modified) libcxx/include/regex (+1-1)

diff --git a/libcxx/include/regex b/libcxx/include/regex
index b814135121321..463fea1638454 100644
--- a/libcxx/include/regex
+++ b/libcxx/include/regex
@@ -3921,7 +3921,7 @@ _ForwardIterator basic_regex<_CharT, _Traits>::__parse_character_escape(
       if (__hd == -1)
         __throw_regex_error<regex_constants::error_escape>();
       __sum = 16 * __sum + static_cast<unsigned>(__hd);
-      // fallthrough
+    [[clang::fallthrough]];
     case 'x':
       ++__first;
       if (__first == __last)

[vegerot]

Question for reviewer: this fallthrough looks strange. Are we sure it shouldn't be a break;?

I should probably change this to a break; and see what tests break...

[frederick-vs-ja]

It seems to me that we should spell the attribute as [[_Clang::__fallthrough__]] to defend against user-defined macros. (As clang is never reserved and fallthrough is reserved only since C++17.)
Or can we use [[__fallthrough__]]?

[philnik777]

Given that there was a fallthrough comment, I'd assume it was intentional. And, as @frederick-vs-ja we should make this [[__fallthrough__]] instead.

[vegerot]

1. done
2. Whether I put a break; or fallthrough here, the tests pass which is a little sus. (thanks to @philnik777 for helping me run the tests)

[vegerot]

question for reviewer: what is the proper etiquette for stacked diffs? Should I open two PRs or have one PR with two commits?

[mordante]

question for reviewer: what is the proper etiquette for stacked diffs? Should I open two PRs or have one PR with two commits?

It's only possible to make stacked reviews for members of the LLVM project since it requires write access to the repository. I prefer you wait uploading the next patch until this one lands. Having two patches mixed causes reviewers to review the same code twice.

[ldionne]

LGTM, this can be merged once CI is green.

[vegerot]

@ldionne Thanks for the +1! CI is green. Are we good to land?

[github-actions]

@vegerot Congratulations on having your first Pull Request (PR) merged into the LLVM Project!

Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR.

Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues.

How to do this, and the rest of the post-merge process, is covered in detail here.

If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again.

If you don't get any reports, no action is required from you. Your changes are working as expected, well done!