Open ncihnegn opened 4 months ago
@llvm/issue-subscribers-clang-modules
Author: None (ncihnegn)
This may be a different issue than https://github.com/llvm/llvm-project/issues/91111. Since that one is complaining redefinition of concept
and this one is about different requires clause
.
I didn't run the reproducer provided by the OP. But construct a similar one:
import std;
#include <iterator>
And the analysis shows, it might be a library issue instead of a compiler issue.
The compiler are complaining about the inconsistent definition from the module and the main file.
template <class _Fp, class... _Its>
requires(indirectly_readable<_Its> && ...) && invocable<_Fp, iter_reference_t<_Its>...>
using indirect_result_t = invoke_result_t<_Fp, iter_reference_t<_Its>...>;
When I look into the issue, I found the AST in the module file shows the &&
used here is the trivial &&
operator but the &&
operator used in main file is a call expr. Then I remember https://github.com/llvm/llvm-project/issues/61643 which is also problematic due to the free standing operator &&
.
Then I tried to comment out using std::operator&&
in valarray.inc
. And it compiles. Now we can explain the reproducer, when the compiler compile #include <iterator>
in the main file, the operator &&
in std module is visible to the current module, then the require clause for indirect_result_t
have a different meaning in the mainfile and so the compiler complains.
CC: @mordante
And if we can't solve the problem quickly, I think it may be helpful to comment out using std::operator&&
and using std::operator||
in valarray.inc
before clang19. I feel valarray is relatively rarely used.
I can reproduce @ChuanqiXu9's reproducer. It seems only <valarray>
exports std::operator&&
and std::operator||
. I agree valarray is rarely used; otherwise there would be more bugreports for this header :-(.
I still prefer to look at a real bug-fix. But having a libc++-19 "hack" doesn't sound too bad; modules are still considered experimental in libc++ and that won't change in LLVM-19.
I tested and it seems more failures occur. __synth_three_way
which IIRC we recently made a lambda again. So I feel a bit reluctant to change this behaviour. We don't have good tests to see what else might break. I think we really should look at a way how libc++ headers and the std module are compatible. @ChuanqiXu9 we talked about that a while ago, maybe we should restart that discussion.
I tested and it seems more failures occur.
__synth_three_way
which IIRC we recently made a lambda again. So I feel a bit reluctant to change this behaviour. We don't have good tests to see what else might break. I think we really should look at a way how libc++ headers and the std module are compatible. @ChuanqiXu9 we talked about that a while ago, maybe we should restart that discussion.
Yeah, given that is a separate issue. Let's talk somewhere else. Maybe discord or another issue. From the perspective of software engineering, I feel it will be better to have more test on this.
Discord is fine. I agree we should have more tests, however there are some concerns of other libc++ developers about adding module tests since they need modifications to existing tests.
Removing from the 19 milestone since it seems this is slipping to the next release.
I tested and it seems more failures occur. __synth_three_way which IIRC we recently made a lambda again.
This may be fixed. Can you try again?
We don't have good tests to see what else might break. I think we really should look at a way how libc++ headers and the std module are compatible. @ChuanqiXu9 we talked about that a while ago, maybe we should restart that discussion.
Oh, I forgot this. Sorry. I hope we can have more tests. But I remember the reason we didn't do this is due to that we don't want this test bring more testing pressure on libc++ folks. So maybe it is better to add a few tests that won't bring you more trouble if possible.
clang version 19.0.0git (https://github.com/llvm/llvm-project 5220b7bea8b01f46e9f7326b9c9a7e550e8451d1)
Similar to #91111.