[PATCH] D145206: [NFC] Properly handle optional minor value for ArchInfo::Version

Zixu Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 3 10:03:42 PST 2023


zixuw added a comment.

In D145206#4167149 <https://reviews.llvm.org/D145206#4167149>, @tmatheson wrote:

> I'm not sure that this is the right fix. The exception indicates that this is being called with ArchInfo objects with invalid VersionTuples, how is that happening? Also could you explain what the issue is with bad_optional_access on older macOS?

Sorry my bad, I wasn't very clear.
Yes we are also looking for a proper fix which guards that test with proper conditions regarding the exceptions support on Darwin. This was just something I find while digging into that issue.
So the libc++ header marks `optional::value` as unavailable on older macOS because of `bad_optional_access` availability. If exception is disabled (which is the case for the rest of LLVM) the availability annotation would still allow older macOS targets to use it, but with an abort instead of throwing `bad_optional_access`. With exceptions enabled by this test, we fail to build the test binary because of the availability.

I was looking into this and noticed that the minor version comparison logic would probably want to use `value_or` anyway so I made this NFC change. If it doesn't seem correct on itself or there are other concerns please let me know!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D145206



More information about the llvm-commits mailing list