Improve the `__is_derived_from_optional` concept

[mordante]

Tested with Clang version 17.0.0 (++20230624042319+ee2bf319bc05-1~exp1~20230624042420.1017)

The issue looks similar to #62943 but that fix is this version of Clang. Testing this code with libc++'s std module

import std;
  
int main(int, char**) {
  int t{3};
  std::optional<int> op{3};
  return op <=> t;
}

Gives the following compiler output

libcxx/test/std/utilities/optional/optional.comp_with_t/compare.three_way.pass.cpp:6:13: error: use of overloaded operator '<=>' is ambiguous (with operand types 'std::optional<int>' and 'int')
    6 |   return op <=> t;
      |          ~~ ^   ~
/llvm/build/generic-module-std-cxx23/include/c++/v1/optional:1612:1: note: candidate function [with _Tp = int, _Up = int]
 1612 | operator<=>(const optional<_Tp>& __x, const _Up& __v) {
      | ^
/llvm/build/generic-module-std-cxx23/include/c++/v1/optional:1612:1: note: candidate function [with _Tp = int, _Up = int]
 1612 | operator<=>(const optional<_Tp>& __x, const _Up& __v) {
      | ^
/llvm/build/generic-module-std-cxx23/include/c++/v1/optional:1612:1: note: candidate function [with _Tp = int, _Up = int]
 1612 | operator<=>(const optional<_Tp>& __x, const _Up& __v) {
      | ^
/llvm/build/generic-module-std-cxx23/include/c++/v1/optional:1612:1: note: candidate function [with _Tp = int, _Up = int]
 1612 | operator<=>(const optional<_Tp>& __x, const _Up& __v) {
      | ^
/llvm/build/generic-module-std-cxx23/include/c++/v1/optional:1612:1: note: candidate function [with _Tp = int, _Up = int]
 1612 | operator<=>(const optional<_Tp>& __x, const _Up& __v) {
      | ^
1 error generated.

error: command failed with exit status: 1

This is a manual reduction of the file libcxx/test/std/utilities/optional/optional.comp_with_t/compare.three_way.pass.cpp

[llvmbot]

@llvm/issue-subscribers-clang-modules

[ChuanqiXu9]

Note that the original reproducer is invalid without modules: https://godbolt.org/z/5aojvWcYW

[ChuanqiXu9]

It turns out the reason for the issue is the same with that we don't merge the lambdas in modules well: https://github.com/llvm/llvm-project/blob/dc6f5c9b588adfe62449a898ebd06a5a09c05439/libcxx/include/optional#L682-L683

If we change the implementation of __is_derived_from_optional to std::is_base_of (or other similar intrinsics), the behavior of the reproducer is expected.

Also personally I feel it looks better to use std::is_base_of (or other similar intrinsics) instead of relying on the lambda tricks even without modules, since std::is_base_of (or other similar intrinsics) uses the compiler built-in intrinsics to do this. It should be more efficient.

[mordante]

Thanks, it seems I was a bit to aggressive removing code, https://godbolt.org/z/h5MxnxhYT works without modules and looks more like the original code.

Thanks for finding the issue. I will investigate whether we can indeed use std::is_base_of instead of this way. I first like to understand why we do it like this. I prefer the type_trait too, not only from an efficiency point of view, but also from a readability point of view.

I'll assign this to myself to keep track of it.

[ChuanqiXu9]

BTW, I'll track the issue in the compiler side in https://github.com/llvm/llvm-project/issues/63544

[mordante]

I had a look and both is_base_of and derived_from take two template arguments. This one takes one argument and deduces the second argument. The trait also matches the wording in the Standard. So I guess it's not trivial to write this differently. Since there is already a patch for Clang I prefer to close this issue and wait for proper Clang support.