search

microsoft / STL

MSVC's implementation of the C++ Standard Library.
Other
10.07k stars 1.49k forks source link

Report EDG bugs #1621

Open StephanTLavavej opened 3 years ago

StephanTLavavej commented 3 years ago

This was originally limited to tracking EDG concepts bugs, but is now unlimited.

:sos: Modules (including Header Units)

There are an unknown number of blocking issues for EDG consuming header units and named modules. Currently, EDG coverage is disabled in the header units test, and hasn't been added to the named modules test:

Click to expand env.lst files: https://github.com/microsoft/STL/blob/a26f4adb1ecf767784bb4e4318093c4579fdb364/tests/std/tests/P1502R1_standard_library_header_units/env.lst#L18-L22 https://github.com/microsoft/STL/blob/a26f4adb1ecf767784bb4e4318093c4579fdb364/tests/std/tests/modules_20_matrix.lst#L18-L20

Help wanted: EDG is looking into the issues with header units, but we can accelerate this work by investigating, reducing, and reporting individual issues.

:white_check_mark: Reported Upstream

:tada: Fixed

:mirror: C1XX Counterparts

StephanTLavavej commented 2 years ago

I am currently seeing 102 unique directories in tests/std failing when I try to enable EDG concepts (with VS 2022 17.2 Preview 1). 90 of these are tests for P0896R4 (ranges).

EDG is currently tracking 16 remaining issues. As that number falls to 0, we should check the tests again.

cpplearner commented 12 months ago

Here are issues discovered when compiling the P0896R4_ranges_iterator_machinery test (all of them are rejects-valid):

  1. EDG fails to handle variable template specializations that differ only in the requires-clause.

:white_check_mark: Reported as VSO-1898880.

Affects _Compile_time_max_size.

Reduced ```c++ template concept C1 = true; template inline constexpr auto max_size = 0; template inline constexpr auto max_size = 0; template requires requires { T::size(); } // error: requires-clause incompatible with variable template inline constexpr auto max_size = 1; ```
  1. EDG thinks that dereferencing a function pointer produces a function. (Should be a reference to function.)

:white_check_mark: Reported as VSO-1898886 (C1XX) and VSO-1898889 (EDG).

Affects ranges::iter_move in unusual scenarios.

Reduced ```c++ void f(); using T = decltype(*f); T r = f; // error: function "r" may not be initialized ```
  1. EDG produces an error for conversion from array to const reference to pointer.

:white_check_mark: Reported as VSO-1898890.

Affects ranges::distance in unusual scenarios.

Reduced ```c++ int some_ints[] = {1, 2, 3}; auto p = static_cast(some_ints); // error: invalid type conversion ```
cpplearner commented 12 months ago

Issues discovered when compiling the P0896R4_views_iota test:

  1. EDG thinks the precedence of <=> operator is between the precedence of < and of ==. (Should be higher.)

:white_check_mark: Already reported as VSO-1161828 on 2020-07-24; also DevCom-10156297 on 2022-09-23. Upstream bug is EDGcpfe/23297.

Reduced ```c++ struct A { int operator<=>(A); }; auto a = A{} <=> A{} > 0; // error: no operator ">" matches these operands ```
  1. EDG has internal compiler error when a range-based for loop over lvalue iota_view is evaluated in a constant expression.

:white_check_mark: Reported as VSO-1898908.

Reduced ```c++ struct _Ioterator { int _Current; constexpr int operator*() const { return 0; } constexpr _Ioterator& operator++() { ++_Current; return *this; } friend constexpr bool operator==(const _Ioterator& _Left, const _Ioterator& _Right) { return _Left._Current == _Right._Current; } }; struct fake_iota_view { using _It = _Ioterator; constexpr _It begin() const { return _It{0}; } constexpr _It end() const { return _It{1}; } }; constexpr bool f() { auto rng = fake_iota_view{}; for (const auto& e : rng) {} return true; } static_assert(f()); ```
cpplearner commented 12 months ago

P0896R4_views_single: EDG does not consider single_view<trivially_copy_assignable_type> to be trivially copy-assignable.

:white_check_mark: Reported as VSO-1898912.

Reduced ```c++ #include template struct box { box& operator=(const box&) requires Trivial = default; constexpr box& operator=(const box&) { return *this; } }; struct fake_single_view { box val; }; static_assert(std::is_trivially_copy_assignable_v>); // OK static_assert(std::is_trivially_copy_assignable_v); // error: static assertion failed ```
cpplearner commented 12 months ago

P0896R4_views_common: Befriending an abbreviated function template makes EDG ignore subsequent members. (This affects difference_type_only_iterator in the test.)

:white_check_mark: Reported as VSO-1898913.

Reduced ```c++ template struct test_type { friend constexpr void f(auto n) noexcept {} T v; }; constexpr void test() { (void) test_type{0}; // error: too many initializer values } ```
cpplearner commented 12 months ago

P0896R4_P1614R2_comparisons: EDG produces a hard error for can_three_way<bool, int>.

:white_check_mark: Reported as VSO-1898915.

Reduced ```c++ #include template concept can_three_way = requires(T const& t, U const& u) { t <=> u; // // error: invalid types ("int" and "bool") for built-in operator<=> }; static_assert(!can_three_way); ```

P0896R4_views_join: EDG ICE with constant-evaluating std::string in _DEBUG mode.

:warning: Semi-reduced with cpplearner/edg-concepts-wip, not yet reported: ``` D:\GitHub\STL\out\x64>type meow.cpp ``` ```cpp #include #include #include #include using namespace std; constexpr string_view arr[2] = {"Hello "sv, "World!"sv}; constexpr string ToString(const int val) { return string{arr[val]}; } int main() { constexpr string_view correct = "Hello World!"sv; static_assert(ranges::equal(views::iota(0, 2) | views::transform(ToString) | views::join, correct)); } ``` ``` D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /MTd /c meow.cpp meow.cpp D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /MTd /c /BE meow.cpp meow.cpp INTERNAL COMPILER ERROR in 'C:\Program Files\Microsoft Visual Studio\2022\Preview\VC\Tools\MSVC\14.38.33030\bin\HostX64\x64\cl.exe' Please choose the Technical Support command on the Visual C++ Help menu, or open the Technical Support help file for more information ```

After I added workarounds for these bugs, EDG successfully compiled all the P0896R4* tests (except that a few tests timed out). It still fails for P2374R4_views_cartesian_product, P2441R2_views_join_with, and maybe others, which I haven't investigated, but in general, the support seems good enough.

cpplearner commented 12 months ago

test_mdspan_support.hpp: EDG needs typename in the parameter list of a requires-expression.

:white_check_mark: Already reported as DevCom-10436970 VSO-1868335 on 2023-08-09.

Reduced ```c++ template auto x = requires(T::ty x) { x; }; // error: nontype "T::ty" is not a type name ```

P0323R12_expected: EDG rejects requires-clause in a definition within a function template.

:white_check_mark: Reported as VSO-1898929.

Reduced ```c++ template void f() { struct S { S() requires Trivial = default; // error: a requires-clause is not allowed here (not a templated function) constexpr S() {} }; } template void f(); ```

P2474R2_views_repeat: EDG rejects parenthesized aggregate initialization in a mem-initializer.

:white_check_mark: Reported as VSO-1898933.

Reduced ```c++ struct forward_tester { forward_tester() = default; constexpr forward_tester(const forward_tester&) {} }; struct tuple_tester { forward_tester y; }; struct t { t(const forward_tester& arg1) : val(arg1) // error: no instance of constructor "tuple_tester::tuple_tester" matches the argument list {} tuple_tester val; }; ```

P0009R18_mdspan_default_accessor: EDG constexpr dynamic allocations can't handle an array of std::string in debug mode

:white_check_mark: Reported as VSO-1898962.

Reduced ``` C:\Temp>type meow.cpp ``` ```cpp // Repros with prod/fe x86chk as of 2023-10-09. // Affects STL test P0009R18_mdspan_default_accessor. #include template struct Array { T elems[N]; }; constexpr bool test() { Array arr{"one", "two", "three"}; return true; } int main() { static_assert(test()); } ``` ``` C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od /c meow.cpp meow.cpp C:\Temp>cl /EHsc /nologo /W4 /std:c++latest /MTd /Od /c /BE meow.cpp meow.cpp "meow.cpp", line 17: error: expression must have a constant value static_assert(test()); ^ "meow.cpp", line 17: note: some dynamic allocations (total number = 2) were not deallocated static_assert(test()); ^ "D:\msvc\binaries\x86chk\inc\xmemory", line 1475: note: allocation occurred here _Ptr = _Unfancy(_Al_.allocate(1)); ^ ```

P2322R6_ranges_alg_fold: failure with vector in _DEBUG mode.

:warning: Semi-reduced with cpplearner/edg-concepts-wip, not yet reported: ``` D:\GitHub\STL\out\x64>type meow.cpp ``` ```cpp #include #include #include #include #include using namespace std; constexpr bool non_dependent() { const vector vec = {10, 20, 30}; const int sum = ranges::fold_left(vec, 0, plus{}); assert(sum == 60); return true; } int main() { static_assert(non_dependent()); } ``` ``` D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /MTd /c meow.cpp meow.cpp D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /MTd /c /BE meow.cpp meow.cpp "meow.cpp", line 19: error: expression must have a constant value static_assert(non_dependent()); ^ "D:\GitHub\STL\out\x64\out\inc\xmemory", line 1357: note: attempt to access expired storage while (*_Pnext && *_Pnext != this) { ^ "D:\GitHub\STL\out\x64\out\inc\xmemory", line 1272: note: called from: _Orphan_me_unlocked_v3(); ^ "D:\GitHub\STL\out\x64\out\inc\algorithm", line 2594: note: called from: _RANGES begin(_Range), _RANGES end(_Range), _STD move(_Init), _Pass_fn(_Func)); ^ "D:\GitHub\STL\out\x64\out\inc\algorithm", line 2594: note: called from: _RANGES begin(_Range), _RANGES end(_Range), _STD move(_Init), _Pass_fn(_Func)); ^ "D:\GitHub\STL\out\x64\out\inc\algorithm", line 2637: note: called from: return _RANGES fold_left_with_iter(_STD forward<_Rng>(_Range), _STD move(_Init), _Pass_fn(_Func)).value; ^ "meow.cpp", line 11: note: called from: const int sum = ranges::fold_left(vec, 0, plus{}); ^ ```
cpplearner commented 12 months ago

P0898R3_concepts:

  1. EDG's __is_convertible_to depends on whether the private members can be accessed in the current context.

:white_check_mark: Reported as VSO-1898937.

Reduced ```c++ template concept converts_to = __is_convertible_to(D*, B*); struct Base {}; class PrivateDerived : private Base { public: void f(); }; void PrivateDerived::f() { // Check these in a member to verify that access doesn't depend on context static_assert(!converts_to); } ```
  1. constructible_from has wrong value.

:white_check_mark: Reported as VSO-1898939.

Reduced ```c++ template concept constructible_from = __is_constructible(_Ty, _ArgTys...); struct Multiparameter { explicit Multiparameter(int); Multiparameter(char) = delete; }; static_assert(!constructible_from); ```
  1. default_initializable<const T> has wrong value.

:white_check_mark: Reported as VSO-1898941.

Reduced ```c++ void* operator new(decltype(sizeof 0) size, void* ptr) noexcept; template concept default_initializable = requires { _Ty{}; ::new (static_cast(nullptr)) _Ty; }; static_assert(!default_initializable); ```
  1. default_initializable<AggregatesExplicitDefault> has wrong value.

:white_check_mark: Reported as VSO-1898944 (C1XX) and VSO-1898945 (EDG).

Reduced ```c++ void* operator new(decltype(sizeof 0) size, void* ptr) noexcept; template concept default_initializable = requires { _Ty{}; ::new (static_cast(nullptr)) _Ty; }; struct ExplicitDefault { explicit ExplicitDefault() = default; }; struct AggregatesExplicitDefault { ExplicitDefault meow; }; static_assert(!default_initializable); ```
  1. std::nullptr_t has relational operators, but it should not. (Affects both totally_ordered and totally_ordered_with.)

:white_check_mark: Reported as VSO-1898947.

Reduced ```c++ template constexpr bool has_less = requires(const T& t) { t < t; }; static_assert(!has_less); ```
StephanTLavavej commented 12 months ago

Thank you!!! :heart_eyes_cat: I've reported 19 bugs (slightly cleaning up some repros) and semi-reduced 2 more.

cpplearner commented 12 months ago

P0009R18_mdspan_mdspan:

  1. Default member initializer is instantiated too eagerly when there's a noexcept defaulted default constructor.

:white_check_mark: Reported as VSO-1900276.

Reduced ```c++ template struct Mdspan_accessor_base { Mdspan_accessor_base() noexcept = default; template constexpr explicit Mdspan_accessor_base(const OtherAccessorPolicy& Acc_) : Acc(Acc_) {} static constexpr int rank = 1; AccessorPolicy Acc = AccessorPolicy(); // error: no instance of constructor "Accessor::Accessor" matches the argument list }; struct Accessor { constexpr explicit Accessor(int id) noexcept {} }; using Mds = Mdspan_accessor_base; static_assert(Mds::rank == 1); ```
  1. Constraint expression cannot be satisfied if it calls a member function of the current class.

:white_check_mark: Reported as VSO-1900278.

Reduced ```c++ template struct mdspan { static constexpr int rank_dynamic() { return V; } constexpr mdspan() requires (rank_dynamic() > 0) {} }; mdspan<1> m; // error: default constructor for "mdspan<1>" is not eligible ```

P2165R4_tuple_like_common_type: Conditional explicit specifier is checked too early in constrained constructor.

:white_check_mark: Reported as VSO-1900279.

Reduced This is similar to Clang bug LLVM-59827. ```c++ template constexpr bool always_false = false; template concept False = always_false; template constexpr bool do_not_instantiate() { static_assert(always_false); return true; } struct T { template explicit(do_not_instantiate()) T(X) { } T(int) { } }; T t(5); ```

P2165R4_tuple_like_operations: Variable template is instantiated too early.

:white_check_mark: Reported as VSO-1900281.

Reduced ```c++ template struct tuple_size; template constexpr size_t tuple_size_v = tuple_size::value; template concept False = false; template struct Tuple_cat2 {}; template using Tuple_cat1 = Tuple_cat2>; template typename Tuple_cat1::Ret tuple_cat(Tuples&& Tpls); template concept CanTupleCat = requires(Tuples tpls) { tuple_cat(tpls); }; static_assert(!CanTupleCat); ```
cpplearner commented 12 months ago

Dev11_0437519_container_requirements: EDG thinks a member does not exist when:

:white_check_mark: Reported as VSO-1900290.

Reduced ```c++ template class move_iterator { public: constexpr move_iterator() = default; template friend bool f(const move_iterator& Left) noexcept(noexcept(Left.Current)); private: Iter Current; }; template struct A {}; template <> struct A<1> { move_iterator j; }; ```

This is unrelated to concepts. Why didn't this test fail before C++20? 🤔

cpplearner commented 12 months ago

P2278R4_views_as_const:

  1. EDG complains about concept depending on itself.

:x: @StephanTLavavej can't repro with VS 2022 17.8 Preview 3 x64, or prod/fe x86chk on 2023-10-11.

Reduced This is similar to Clang bug LLVM-62096. ```c++ #include template concept Operator = std::is_nothrow_copy_constructible_v; struct AnyOperator { template explicit AnyOperator(OP op) noexcept {} /// Copy constructor AnyOperator(const AnyOperator&) noexcept = default; }; static_assert(Operator); ```
  1. Internal compiler error involving pointer arithmetic.

:white_check_mark: Reported as VSO-1900291.

Reduced ```c++ template constexpr It prev(It Where) { --Where; return Where; } struct fake_single_view { int value_; constexpr const int* end() const noexcept { return &value_ + 1; } }; bool test_one(fake_single_view rng) { auto r2 = rng; (void) prev(r2.end()); return true; } int main() { test_one(fake_single_view(333)); } ```

P2374R4_views_cartesian_product:

  1. EDG fails to handle a requires-clause that involves a local type alias.

:white_check_mark: Reported as VSO-1900292.

Reduced ```c++ template constexpr int max_size = 1; template constexpr int get_max_size() { using S = decltype(max_size); if constexpr (requires(S val) { bit_width(val); }) { return bit_width(max_size); } else { return -1; } } template int get_max_size(); ```
  1. EDG rejects constrained alias template in a pack expansion, when the pack is empty.

:white_check_mark: Reported as VSO-1900293.

Reduced ```c++ template void begin(T& t); template concept range = requires(T& t) { begin(t); }; template using all_t = int; template concept CartesianIsSizedSentinel = true; template constexpr void test_one(Rest&&... rest) { (void)(CartesianIsSizedSentinel...>); // error: template constraint not satisfied } constexpr void instantiation_test() { test_one(); } ```

P2441R2_views_join_with: Constrained default constructor causes problems.

:white_check_mark: Reported as VSO-1900294.

Reduced This is similar to Clang bug LLVM-60293. ```c++ #include template concept destructible = __is_nothrow_destructible(T); template concept constructible_from = destructible && __is_constructible(T, Args...); template concept default_initializable = constructible_from && requires { T{}; ::new (static_cast(nullptr)) T; }; struct NoDefaultCtor { explicit NoDefaultCtor(int); }; template struct Wrapper { T val{}; }; template struct ConstrainedWrapper { ConstrainedWrapper() requires default_initializable = default; ConstrainedWrapper(int); T val{}; }; using Wrapped = Wrapper; using CW = decltype(ConstrainedWrapper{0}); ```

P0898R3_concepts: EDG thinks that conditional-expression has function type when it should have reference-to-function type.

:white_check_mark: Reported as VSO-1900296.

Reduced ```c++ template inline constexpr bool is_same_v = false; template inline constexpr bool is_same_v = true; using FuncType = int(int); FuncType& f(); using common_ref = decltype(true ? f() : f()); static_assert(is_same_v); ```

These should be all.

And here are tentative workarounds for all these bugs.

StephanTLavavej commented 12 months ago

:heart_eyes_cat: Thanks again!! I reported 10 more bugs (I had to fix a few repros, e.g. one said rank_dynamic instead of actually calling rank_dynamic(), another was using a Clang intrinsic __is_same but I file bugs with MSVC vs. EDG comparisons so I had to extract is_same_v, and another assumed your modified <concepts> so I extracted the relevant machinery).

However, I was completely unable to reproduce one bug, noted above with :x:. Can you double-check that one?

cpplearner commented 11 months ago

EDG on godbolt fails to compile the LLVM-62096 test case, but cl /BE works. Maybe it's actually a different bug.

Actually cl /BE rejects this, reduced from P2278R4_views_as_const:

:white_check_mark: Reported as VSO-1901430.

Reduced: ```c++ #include template concept can_move_construct = std::is_move_constructible_v; template concept has_data = requires(T t) { t.data(); }; template struct as_const_view { explicit as_const_view(V); auto data() { static_assert(can_move_construct>); } }; struct span { template requires has_data span(R&&); }; static_assert(can_move_construct>); ```
cpplearner commented 9 months ago
  • VSO-1900291 EDGcpfe/26751 EDG ICE involving pointer arithmetic
    • EDG can repro a spurious error here, but not the ICE that repros in VS. They hope that it's the same underlying issue.

The ICE still exists in 17.9 Preview 2. Also note that the reduced test case makes EDG eccp (on godbolt.org) segfault: https://godbolt.org/z/Ys3rd1c8f.

cpplearner commented 9 months ago
  • VSO-1900292 EDG fails to handle a requires-clause that involves a local type alias

This is fixed, but now EDG produces a wrong value for such a requires-clause (false when it should be true):

template<class T>
constexpr bool always_true = true;

template <class T, bool = always_true<T>>
constexpr int bit_width(T) { return 1; }

template<class T>
constexpr int max_size = 1;

template<class T>
constexpr int get_max_size() {
    using S = decltype(max_size<T>);
    if constexpr (requires(S val) { ::bit_width(val); }) {
        return ::bit_width(max_size<T>);
    } else {
        return -1;
    }
}

static_assert(get_max_size<int>() == 1);  // error: static assertion failed
CaseyCarter commented 8 months ago

  • VSO-1900291 EDGcpfe/26751 EDG ICE involving pointer arithmetic

    • EDG can repro a spurious error here, but not the ICE that repros in VS. They hope that it's the same underlying issue.

The ICE still exists in 17.9 Preview 2. Also note that the reduced test case makes EDG eccp (on godbolt.org) segfault: https://godbolt.org/z/Ys3rd1c8f.

I can still repro this with the most-recent compiler drop we got from the Edge team in early January, which AFAIK should contain the fix. I've poked VSO-1900291 and asked them to reverify that it is fixed.

CaseyCarter commented 8 months ago

I can still repro this with the most-recent compiler drop we got from the Edge team in early January, which AFAIK should contain the fix. I've poked VSO-1900291 and asked them to reverify that it is fixed.

The Edge team investigated and determined that they sent the wrong repro to EDG for VSO-1900291. They have reactivated the bug and are trying again =)

cpplearner commented 8 months ago

generator: EDG instantiates the wrong overload of promise_type::operator new.

Partially reduced The use of `coroutine_handle<_Gen_promise_base>::from_promise` seems to matter. Removing it makes the error disappear. ```c++ #include using namespace std; struct allocator_arg_t {}; template class _Promise_allocator { private: static void* _Allocate(_Alloc _Al, const size_t _Size); public: static void* operator new(const size_t _Size) requires false { return _Allocate(_Alloc{}, _Size); } template requires true static void* operator new(size_t _Size, const _This&, allocator_arg_t, const _Alloc2& _Al, const _Args&...); static void operator delete(void* _Ptr, size_t _Size) noexcept; }; class _Gen_promise_base { public: suspend_always initial_suspend() noexcept { return {}; } auto final_suspend() noexcept { return suspend_always{}; } void await_transform() = delete; void return_void() const noexcept {} void unhandled_exception() { throw; } coroutine_handle<_Gen_promise_base> _Top = coroutine_handle<_Gen_promise_base>::from_promise(*this); }; template class generator { public: struct __declspec(empty_bases) promise_type : _Promise_allocator<_Alloc>, _Gen_promise_base { generator get_return_object() noexcept; }; }; template struct stateful_alloc { explicit stateful_alloc(int dom) noexcept; }; void static_allocator_test() { { auto g = [](allocator_arg_t, stateful_alloc) -> generator> { co_return; }; (void) g(allocator_arg_t{}, stateful_alloc{42}); } } ``` Error message: ``` "test-std-generator.cpp", line 15: error: no instance of constructor "stateful_alloc::stateful_alloc [with T=char]" matches the argument list return _Allocate(_Alloc{}, _Size); ^ detected during instantiation of "void *_Promise_allocator<_Alloc>::operator new(size_t _Size) [with _Alloc=stateful_alloc]" ```
StephanTLavavej commented 8 months ago

Thanks for extracting these repros! :heart_eyes_cat:

I've further reduced and reported VSO-1951794 "EDG produces a wrong value for a requires-clause that involves a local type alias".

Click to expand reduced repro: ``` D:\GitHub\STL\out\x64>type meow.cpp ``` ```cpp template constexpr bool always_true = true; template > constexpr void func(T) {} template constexpr int thing = 1; template constexpr bool can_call_func_with_thing() { using ThingType = decltype(thing); return requires(ThingType val) { ::func(val); }; } static_assert(can_call_func_with_thing()); ``` ``` D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp meow.cpp D:\GitHub\STL\out\x64>clang-cl /EHsc /nologo /W4 /std:c++latest /c meow.cpp D:\GitHub\STL\out\x64>cl /EHsc /nologo /W4 /std:c++latest /c /BE meow.cpp meow.cpp "meow.cpp", line 16: error: static assertion failed static_assert(can_call_func_with_thing()); ^ ```

I've also reported VSO-1951821 "EDG instantiates the wrong overload of promise_type::operator new for generator machinery". Both are recorded in the list above.