cmake: Restrict MSVC-specific workaround to affected versions #32499

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:250514-msvc-bug changing 1 files +9 −4
  1. hebasto commented at 2:56 pm on May 14, 2025: member
    The recent MSVC version 17.14 has fixed a bug, so we can now enable compilation of fuzz/utxo_snapshot.cpp with the updated compiler.
  2. cmake: Restrict MSVC-specific workaround to affected versions 6779430327
  3. hebasto added the label Windows on May 14, 2025
  4. hebasto added the label Build system on May 14, 2025
  5. DrahtBot commented at 2:56 pm on May 14, 2025: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32499.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK sipsorcery

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #31507 (build: Use clang-cl to build on Windows natively by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  6. maflcko commented at 2:59 pm on May 14, 2025: member
    No objection, but I guess it also seems fine to wait a few weeks until GitHub bumps their Windows image and then just bump the msvc minimum required version.
  7. hebasto commented at 3:02 pm on May 14, 2025: member

    No objection, but I guess it also seems fine to wait a few weeks until GitHub bumps their Windows image and then just bump the msvc minimum required version.

    Do we really need to force Windows developers to update their toolchains in this case?

  8. maflcko commented at 3:20 pm on May 14, 2025: member

    Do we really need to force Windows developers to update their toolchains in this case?

    I expect the number of Windows devs is still small enough to just ping and ask them, when the time has come.

  9. hebasto commented at 5:07 pm on May 14, 2025: member

    Do we really need to force Windows developers to update their toolchains in this case?

    I expect the number of Windows devs is still small enough to just ping and ask them, when the time has come.

    Friendly ping @davidgumberg @hodlinator @sipsorcery :)

  10. sipsorcery commented at 8:01 pm on May 14, 2025: contributor

    utACK 677943032785253ab268e51c9d37fbacc1483568.

    17.14 is still in preview so can’t test yet (would rather avoid the >10GB download and install for the sake of a week or two).

  11. hebasto commented at 9:05 pm on May 14, 2025: member

    17.14 is still in preview so can’t test yet (would rather avoid the >10GB download and install for the sake of a week or two).

    From https://learn.microsoft.com/en-us/visualstudio/releases/2022/release-notes:

    17.14 … was released on May 13, 2025.

  12. sipsorcery commented at 9:21 pm on May 14, 2025: contributor

    Just checked again (1 hour later) and installing 17.14 now.

    Seems it was just my VS Installer instance being crap and telling me there were no new updates and offering 17.14 as a preview.

  13. hodlinator commented at 11:28 am on May 15, 2025: contributor

    nit: Should PR description include “ref #31303”?

    Tried removing the compiler_has_bug-logic completely and building with the version I had installed: 17.13.4

    0cmake -B build --preset vs2022 -DVCPKG_INSTALL_OPTIONS="--x-buildtrees-root=C:\vcpkg" -DBUILD_FUZZ_BINARY=ON -DBUILD_GUI=OFF
    1cmake --build build --config Release -j16
    

    …did not encounter the internal compiler error?

    Couldn’t repro the ICE from https://developercommunity.visualstudio.com/t/C1001:-Internal-compiler-error-with-st/10830350 either:

    0> cl.exe /std:c++20 /c ice_demo.cpp
    1Microsoft (R) C/C++ Optimizing Compiler Version 19.43.34809 for x86
    2Copyright (C) Microsoft Corporation.  All rights reserved.
    3
    4ice_demo.cpp
    5
    6>
    
  14. in src/test/fuzz/CMakeLists.txt:13 in 6779430327
     8+# that causes an internal compiler error.
     9+# See:
    10+#  - https://github.com/bitcoin/bitcoin/issues/31303
    11+#  - https://developercommunity.visualstudio.com/t/C1001:-Internal-compiler-error-with-st/10830350
    12+# Fixed in Visual Studio 2022 version 17.14.
    13+set(compiler_has_bug "$<AND:$<CXX_COMPILER_ID:MSVC>,$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,19.42>,$<VERSION_LESS:$<CXX_COMPILER_VERSION>,19.44>>")
    


    davidgumberg commented at 8:31 pm on May 15, 2025:
    Question: Where do these version numbers come from? Is there a reference to correlate the Visual Studio version with the MSVC versions, or did you get these by testing manually?

    davidgumberg commented at 8:33 pm on May 15, 2025:
  15. davidgumberg commented at 6:31 am on May 16, 2025: contributor

    Tried removing the compiler_has_bug-logic completely and building with the version I had installed: 17.13.4

    […]

    …did not encounter the internal compiler error?

    My guess is either that a hotfix was pushed for 19.43, or that this bug was already fixed in 19.43, and the labels / bot response on the msvc bugtracker are inaccurate or canned, don’t have an MSVC in front of me, but using Godbolt’s MSVC 19.42.34441 the minimal reproduction causes internal error C1001, and MSVC 19.43.34810 does not.

  16. hebasto commented at 7:04 am on May 16, 2025: member
    Closing in favour of #32525.
  17. hebasto closed this on May 16, 2025


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-05-20 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me