STL icon indicating copy to clipboard operation
STL copied to clipboard

Enable `__cpp_lib_concepts` for EDG, part 1

Open cpplearner opened this issue 1 year ago • 1 comments

This PR makes __cpp_lib_concepts defined in C++20 mode for all supported compilers, after adding workarounds for various EDG concepts bugs. I have opened two follow-up PRs for non-essential and largely mechanical clean-ups.

  • Part 2 (#4297) replaces concepts_20_matrix and concepts_latest_matrix with usual_20_matrix and usual_latest_matrix respectively.
  • Part 3 (#4298) replaces defined(__cpp_lib_concepts) in preprocessing conditions with _HAS_CXX20, and then eliminates some of them where they are redundant.

cpplearner avatar Jan 04 '24 11:01 cpplearner

Oh wow, this is amazing! Thanks!

CaseyCarter avatar Jan 04 '24 13:01 CaseyCarter

/pr review

cpplearner avatar Jan 23 '24 16:01 cpplearner

EDG is failing the new triviality tests in P0323R12_expected added by #4271. We need to merge main, minimize a repro to file, and disable the failing cases. (I'd do this now, but I'd rather sleep.)

CaseyCarter avatar Jan 31 '24 11:01 CaseyCarter

  • Pushed a source-conflict-free merge with main to pick up #4271.
  • Fully reduced and reported VSO-1949451 "EDG concepts rejects std::expected trying to propagate triviality of assignment operations".
  • Recorded that in tracking issue #1621.
  • Disabled the affected top-level scenarios for EDG in P0323R12_expected.

They were TrivialityScenario4 "Only destruction and move construction/assignment are trivial" and TrivialityScenario6 "All operations are trivial", revealing @CaseyCarter's deep wisdom in https://github.com/microsoft/STL/pull/4271#issuecomment-1882111249 of testing these scenarios. :heart_eyes_cat:

StephanTLavavej avatar Jan 31 '24 12:01 StephanTLavavej

I'm mirroring this to the MSVC-internal repo - please notify me if any further changes are pushed.

StephanTLavavej avatar Feb 01 '24 01:02 StephanTLavavej

I've pushed additional commits:

  • Attempt to avoid timeouts in P0896R4_ranges_alg_find_end.
    • Encountered in the internal test harness, probably because the "checked" compiler consumes more memory.
  • Drop "assert bug so we'll notice when it's fixed".
    • I know this had worthy intentions, but it's a huge headache when either MSVC or EDG devs are trying to test their fixed compilers.
  • Properly detect MSVC, part 1.
    • DevCom-10265237 "Parameter pack size mismatch doesn't make the require-expression false (regression)" is an MSVC bug.
    • This preprocessor logic is guarding an additional constraint that MSVC needs, so I'm updating the #endif comment to ^^^ workaround ^^^.
    • Previously, I believe this code was mentally saying, "we're in concepts code, so we only need to distinguish Clang from MSVC". This is no longer the case. Now we need !defined(__clang__) && !defined(__EDG__) to prevent EDG from using a workaround that it doesn't need.
  • Properly detect MSVC, part 2.
    • DevCom-1691516 "std::vector::emplace_back does not work for a class with default initializer" is an MSVC bug.
    • Curiously, this preprocessor logic was checking only #ifdef __EDG__ to distinguish it from MSVC. (I think it might have been written before Clang supported construct_at()?)
    • Instead, we need to check #if defined(__clang__) || defined(__EDG__) and send them to the "no workaround" case. This prevents Clang from using a workaround that it doesn't need.

I have comment cleanups for a followup PR but I didn't want to further randomize this one when it's so close to landing.

StephanTLavavej avatar Feb 01 '24 10:02 StephanTLavavej

:heart_eyes_cat: :tada: :cat2: :rocket: :cake:

Thank you @cpplearner and @CaseyCarter for the massive amount of work needed to get to this long-awaited day!

StephanTLavavej avatar Feb 01 '24 23:02 StephanTLavavej