Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[C++20] [Modules] Including <valarray> in GMF in front of other headers may violate ODR Checking #61643

Open
ChuanqiXu9 opened this issue Mar 23, 2023 · 7 comments
Labels
clang:modules C++20 modules and Clang Header Modules libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@ChuanqiXu9
Copy link
Member

ChuanqiXu9 commented Mar 23, 2023

We meet the problem twice now (#61642 and #61150) so it may make sense to create a summary issue for this to avoid wider confusion.

Here is the reproducer:

// a.cppm
module;
#include <ranges>
export module a;

// b.cppm
module;
#include <valarray>
#include <ranges>
export module b;
import a;

Compile it with:

clang++ -std=c++2b $(STL_PATH) a.cppm --precompile -o a.pcm
clang++ -std=c++2b $(STL_PATH) b.cppm -fmodule-file=a=a.pcm --precompile -o b.pcm

(STL_PATH refer to libcxx HEAD in my reproducer). And it gives error diagnostics:

In module 'a' imported from b.cppm:5:
/home/chuanqi.xcq/workspace.xuchuanqi/llvm-project-for-work/build/61642/../../build_libcxx/include/c++/v1/variant:1743:16: error: 'std::__throw_if_valueless' has different definitions in different modules; definition in module 'a.<global>' first difference is function body
constexpr void __throw_if_valueless(_Vs&&... __vs) {
~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../build_libcxx/include/c++/v1/variant:1743:16: note: but in '' found a different body
constexpr void __throw_if_valueless(_Vs&&... __vs) {
~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In module 'a' imported from b.cppm:5:
/home/chuanqi.xcq/workspace.xuchuanqi/llvm-project-for-work/build/61642/../../build_libcxx/include/c++/v1/tuple:1562:1: error: 'std::__tuple_compare_three_way' has different definitions in different modules; definition in module 'a.<global>' first difference is function body
__tuple_compare_three_way(const tuple<_Tp...>& __x, const tuple<_Up...>& __y, index_sequence<_Is...>) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../../build_libcxx/include/c++/v1/tuple:1562:1: note: but in '' found a different body
__tuple_compare_three_way(const tuple<_Tp...>& __x, const tuple<_Up...>& __y, index_sequence<_Is...>) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
In module 'a' imported from b.cppm:5:
/home/chuanqi.xcq/workspace.xuchuanqi/llvm-project-for-work/build/61642/../../build_libcxx/include/c++/v1/__ranges/zip_view.h:494:40: error: 'std::__1::ranges::views::__zip::__fn::operator()' from module 'a.<global>' is not present in definition of 'std::__1::ranges::views::__zip::__fn' provided earlier
  _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Ranges&&... __rs) const
                                       ^
../../build_libcxx/include/c++/v1/__ranges/zip_view.h:491:40: note: declaration of 'operator()' does not match
  _LIBCPP_HIDE_FROM_ABI constexpr auto operator()() const noexcept { return empty_view<tuple<>>{}; }
                                       ^
../../build_libcxx/include/c++/v1/__ranges/zip_view.h:494:40: note: declaration of 'operator()' does not match
  _LIBCPP_HIDE_FROM_ABI constexpr auto operator()(_Ranges&&... __rs) const

According to my analysis, this is caused by the free standing operators in <valarray> affected the operator in the require clause. We can find the example in #61150. And the compilation completes after I remove the free standing operator in valarray. Here is the example: https://reviews.llvm.org/D146695

Although I add the libcxx tag, I am not sure that this is libcxx bug. I just add it to CC the libcxx folks. Since I met this in libstdc++ too. Maybe this is a defect in the spec.

@ChuanqiXu9 ChuanqiXu9 added libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. clang:modules C++20 modules and Clang Header Modules labels Mar 23, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 23, 2023

@llvm/issue-subscribers-clang-modules

@mordante
Copy link
Member

Agreed I noticed this in both libc++ and libstdc++ using the reproducer in #61642. Commenting out the 3 operator&& overloads in libc++ "solved" the issue.

I have not checked the Standard what the required behaviour should be.

I wonder whether the same issue can occur with the global operator||.

@ChuanqiXu9
Copy link
Member Author

While we need to check the standard to know what is the "correct" thing, I believe the global || will have similar impact.

@ChuanqiXu9
Copy link
Member Author

ChuanqiXu9 commented Mar 23, 2023

While I am not familiar with standard library, I feel like the implementation is not the same with the standard:

The standard says:

template<class T> valarray<bool> operator&&(const valarray<T>&, const valarray<T>&);
template<class T> valarray<bool> operator&&(const valarray<T>&,
                                              const typename valarray<T>::value_type&);
template<class T> valarray<bool> operator&&(const typename valarray<T>::value_type&,
                                              const valarray<T>&);

And the libcxx implement it like:

template<class _Expr1, class _Expr2>
inline _LIBCPP_INLINE_VISIBILITY
typename enable_if
<
    __is_val_expr<_Expr1>::value && __is_val_expr<_Expr2>::value,
    __val_expr<_BinaryOp<logical_and<typename _Expr1::value_type>, _Expr1, _Expr2> >
>::type
operator&&(const _Expr1& __x, const _Expr2& __y) { ... }

template<class _Expr>
inline _LIBCPP_INLINE_VISIBILITY
typename enable_if
<
    __is_val_expr<_Expr>::value,
    __val_expr<_BinaryOp<logical_and<typename _Expr::value_type>,
               _Expr, __scalar_expr<typename _Expr::value_type> > >
>::type
operator&&(const _Expr& __x, const typename _Expr::value_type& __y) { ... }

template<class _Expr>
inline _LIBCPP_INLINE_VISIBILITY
typename enable_if
<
    __is_val_expr<_Expr>::value,
    __val_expr<_BinaryOp<logical_and<typename _Expr::value_type>,
               __scalar_expr<typename _Expr::value_type>, _Expr> >
>::type
operator&&(const typename _Expr::value_type& __x, const _Expr& __y) { ... }

I feel like the libcxx's implementations is more relax than the spec.

@mordante
Copy link
Member

I tried with a simpler operator& and still ran into the same issues. I noticed something in the naming of operator&. Its qualified name is std::__1::operator&. For other operators their qualified name is std::operator@. The difference is due to cstddef. This header specifies operator& (and several others) for std::byte.

In libc++ most declarations are in the inline namespace std::__1. But a few are in the namespace std::. (This is done for binary compatibility with libstdc++.)

std::complex is in the inline namespace and std::byte is not. Can this be something that causes this issue?

Note that operator| has the same issue, so maybe #61912 is related to this issue.

@ChuanqiXu9
Copy link
Member Author

ChuanqiXu9 commented Apr 13, 2023

I tried with a simpler operator& and still ran into the same issues. I noticed something in the naming of operator&. Its qualified name is std::__1::operator&. For other operators their qualified name is std::operator@. The difference is due to cstddef. This header specifies operator& (and several others) for std::byte.

In libc++ most declarations are in the inline namespace std::__1. But a few are in the namespace std::. (This is done for binary compatibility with libstdc++.)

std::complex is in the inline namespace and std::byte is not. Can this be something that causes this issue?

Note that operator| has the same issue, so maybe #61912 is related to this issue.

I am not sure if I misunderstand this. But this issue is about operator && instead of operator &, isn't it?


I feel like #61912 is a real compiler bug from the diagnostic message.

@mordante
Copy link
Member

ah yes sorry I looked at the wrong operator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:modules C++20 modules and Clang Header Modules libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

No branches or pull requests

3 participants