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

Toolset update: VS 2019 16.10 Preview 4 #1920

Merged
merged 28 commits into from
May 21, 2021

Conversation

StephanTLavavej
Copy link
Member

@StephanTLavavej StephanTLavavej commented May 13, 2021

  • Update the toolset to VS 2019 16.10 Preview 4 and Python 3.9.5.
    • I'm adding a step to create-vmss.ps1 that sleeps for 60 seconds after rebooting the VM, before running sysprep.ps1. This is because the script failed twice, saying that something else was busy. Each failure costs about 30 minutes. I don't understand the root cause, but I'm hoping that this sleep should make the script more robust, even though this is not a good technique.
  • This also removes an enormous number of compiler bug workarounds. Many were fixed before 16.10p4 but we were incredibly busy and didn't have time to react.
  • DevCom-952724 "MSVC allows default-initialized const int in new-expression" was mostly fixed, but we still need the workaround for:
    #if defined(MSVC_INTERNAL_TESTING) || defined(__clang__) || defined(__EDG__) // TRANSITION, DevCom-952724
    STATIC_ASSERT(!default_initializable<S const>);
    #endif // TRANSITION, DevCom-952724

    As this is passing with MSVC_INTERNAL_TESTING, I didn't report a new bug - I assume we just need a followup fix that didn't get into Preview 4. We can check again in the future and report a new bug if it persists.
  • We need to unskip bit_ceil.fail.cpp, which now fails to compile (as desired!) with:
    bit_ceil.fail.cpp(39): error C2131: expression did not evaluate to a constant
    bit(51): note: failure was caused by the right operand of '<<' being too large (undefined behavior)
    
    I've left <bit>: bit_ceil(T(-1)) should not be a constant expression #1595 open because there may be remaining work to do.
  • Enabling commented-out coverage for WG21-P1065 in Dev11_0535636_functional_overhaul (fixing mentions of a nonexistent sp) revealed an MSVC compiler bug which I reported as DevCom-1419425 "constexpr PMD emits bogus error C2131".
    • This is the only cleanup that isn't a compiler workaround removal; it was TRANSITION because we implemented constexpr invoke() before constexpr ref(). It ended up being a compiler workaround addition, though! 😺
  • DevCom-1337958 "allowing narrowing conversions during template evaluation with /BE" was mostly fixed; I reported VSO-1327220 "EDG doesn't detect narrowing conversions from a user-defined type to a pointer type to bool".
  • When removing workarounds in product code, especially workarounds for EDG, I'm a little concerned about breaking CUDA. All of the product changes here are C++17/20 so there's no CUDA 10.1 Update 2 impact.
  • VSO-590216 "REPORTED: EDG should implement CWG 1518 for IsImplicitlyDefaultConstructible" wasn't completely fixed; I reported VSO-1327248 "EDG still doesn't handle IsImplicitlyDefaultConstructible".
  • VSO-1270011 "REPORTED: [Feedback] ICE during constant evaluation with /BE" wasn't completely fixed; I reported VSO-1327238 "EDG still ICEs with library support for constexpr containers".
  • VSO-1307828 "Standard Library Header Units: ICE with queue's friend bool operator==" was fixed, but I'd like to keep the _Get_container() workaround permanently. It's much simpler than the previous friendship scheme, and I'm willing to pay the debug codegen cost of a helper function here (we sometimes spend code complexity to improve debug codegen, but this is too much for too obscure of an scenario).

StephanTLavavej and others added 27 commits May 11, 2021 18:21
DevCom-952724 "MSVC allows default-initialized const int in new-expression".

Still need the workaround for STATIC_ASSERT(!default_initializable<S const>).
This now fails to compile with:

bit_ceil.fail.cpp(39): error C2131: expression did not evaluate to a constant
bit(51): note: failure was caused by the right operand of '<<' being too large (undefined behavior)
Reported DevCom-1419425 "constexpr PMD emits bogus error C2131".
Reported VSO-1327220 "EDG doesn't detect narrowing conversions
from a user-defined type to a pointer type to bool".
This is C++17 CTAD, no CUDA 10.1 Update 2 impact.
Reported VSO-1327248 "EDG still doesn't handle IsImplicitlyDefaultConstructible".
variant is C++17, no CUDA 10.1 Update 2 impact.
This is C++20 spaceship, no CUDA impact.
This is C++20, no CUDA impact.
Reported VSO-1327238 "EDG still ICEs with library support for constexpr containers".
This is C++20 chrono, so no CUDA impact.
It's simpler than the old friendship scheme.
@StephanTLavavej StephanTLavavej added the infrastructure Related to repository automation label May 13, 2021
@StephanTLavavej StephanTLavavej requested a review from a team as a code owner May 13, 2021 02:10
Copy link
Contributor

@fsb4000 fsb4000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, update to EDG 6.2 particially helped with tests and Intellesense. Looking forward for upgrade to EDG 6.3, which support "using enums".

Bad that python 3.9.5 broke that workaround(#1906), so now I have to relogin with new language interface if I want to run tests and after finish relogin back. But at least no reboot needed, just relogin, it is fast. Or just use python 3.9.4.

By the way where do we require the python version?
It seems now we require only 3.9 without additional number.

cmake -G Ninja -S . -B out\build\x86
....
-- Found Python: C:/Python39/python.exe (found suitable version "3.9.5", minimum required is "3.9") found components: Interpreter
...

@StephanTLavavej
Copy link
Member Author

@fsb4000

By the way where do we require the python version?
It seems now we require only 3.9 without additional number.

That's correct, it's required here:

find_package(Python "3.9" REQUIRED COMPONENTS Interpreter)

I believe we could require 3.9.5 or later, if we wanted to reduce variation between configurations.

@StephanTLavavej StephanTLavavej changed the title Toolset update: VS 2019 16.10 Preview 3 Toolset update: VS 2019 16.10 Preview 4 May 19, 2021
@StephanTLavavej
Copy link
Member Author

@CaseyCarter FYI, I pushed an update to Preview 4 after you approved, no product code changes.

azure-devops/create-vmss.ps1 Show resolved Hide resolved
@StephanTLavavej StephanTLavavej self-assigned this May 20, 2021
@StephanTLavavej StephanTLavavej merged commit 29bd5bd into microsoft:main May 21, 2021
@StephanTLavavej StephanTLavavej deleted the toolset_update branch May 21, 2021 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Related to repository automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants