Use value_or instead of value for checking minor versions in
ArchInfo::implies.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This fixes the checking logic, and also works around a build failure with clang/unittest/Interpreter/ExceptionTests which enables exceptions. std::optional::value() throws std::bad_optional_access, which won't build if targeting older versions of macOS
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!
For anyone reading, there's more context here about std::optional::value(): https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716
I'll add the assert back in D145371.