root icon indicating copy to clipboard operation
root copied to clipboard

[MSVC] Root is failed with error G694476FC: static_assert failed "Unexpected size"

Open NEIL-smtg opened this issue 1 year ago • 13 comments
trafficstars

Check duplicate issues.

  • [x] Checked for duplicates

Description

We've been using MSVC to build ROOT, and we've recently encountered the following error:

error G694476FC: static_assert failed "Unexpected size"

Our developer has suggested the compiler that was used needs to implement the Defect Report https://cplusplus.github.io/CWG/issues/2518.html (which has been implemented by MSVC and Clang).

Reproducer

Repro step:

  1. Open "x64 Native Tools Command Prompt for VS 2022"
  2. git clone https://github.com/root-project/root.git
  3. mkdir root\build_amd64
  4. cd /d root\build_amd64
  5. set VSCMD_SKIP_SENDTELEMETRY=1 & "C:\Program Files\Microsoft Visual Studio\2022\Enterprise\Common7\Tools\VsDevCmd.bat" -host_arch=amd64 -arch=amd64
  6. python.exe -m pip install pytest 2>&1
  7. cmake -G "Visual Studio 17 2022" -A x64 -DCMAKE_SYSTEM_VERSION=10.0.22621.0 -Dtesting=ON -Droottest=ON -Droofit=off .. 2>&1
  8. msbuild /m /p:Platform=x64 /p:Configuration=Release ALL_BUILD.vcxproj /t:Rebuild 2>&1

Expected result:

Build succesfully.

ROOT version

We are using the latest commit.

Installation method

git

Operating system

Windows server 2022 data center

Additional context

Detailed log: Build.log

NEIL-smtg avatar Apr 24 '24 07:04 NEIL-smtg

So I cannot reproduce the error with the recipe you give. But looking at you log file, I see you set more environment variables. That is:

set _CL_= /Bcapture_repro C:\a\_work\_temp\rwc_project_logs\ROOT\preprocessed_repro_build & set _LINK_= /onfailrepro:C:\a\_work\_temp\rwc_project_logs\ROOT\link_repro_build

Without those variables the build works just fine. This is maybe related to https://github.com/root-project/root/issues/9445 I'm now trying with the same variables to see if it fails the same

bellenot avatar Apr 25 '24 11:04 bellenot

So I tried with:

cmake -G "Visual Studio 17 2022" -A x64 -Dtesting=ON -Droottest=ON -Droofit=off ..\..\..\git\master
set VSCMD_SKIP_SENDTELEMETRY=1 & "C:\Program Files\Microsoft Visual Studio\2022\Community\Common7\Tools\VsDevCmd.bat" -host_arch=amd64 -arch=amd64 & set _CL_= /Bcapture_repro C:\project_logs\ROOT\preprocessed_repro_build & set _LINK_= /onfailrepro:C:\project_logs\ROOT\link_repro_build
msbuild /m /p:Platform=x64 /p:Configuration=Release ALL_BUILD.vcxproj /t:Rebuild

And it worked...

bellenot avatar Apr 25 '24 12:04 bellenot

Neil omitted an important bit of info - this involves a very recent change to MSVC's STL, https://github.com/microsoft/STL/pull/4591 . To reproduce it, you'll either have to build and consume our open-source STL, or wait for VS 2022 17.11 Preview 2 to ship (which will contain this commit). We changed the STL to say static_assert(false) instead of static_assert(_Always_false<SomeType>).

StephanTLavavej avatar Apr 25 '24 19:04 StephanTLavavej

@StephanTLavavej OK, thanks for the info. But then what does that mean for ROOT? It will not build anymore with the upcoming version 17.11 of Visual Studio (again!!) ? What would be required to fix the issue on our side?

bellenot avatar Apr 26 '24 06:04 bellenot

Correct. To fix this, implement CWG-2518 in Cling (if that's the tool that's emitting this error), allowing static_assert(false) to work in templates. MSVC and Clang have already implemented this Core DR.

StephanTLavavej avatar Apr 26 '24 15:04 StephanTLavavej

Clang have already implemented this Core DR.

Do you happen to know which version this appeared in? (Or better yet which Merge Request added it). Cling uses Clang underneath; currently version 16; Moving the Clang version used is a long process but backporting a narrow Merge Request might be possible.

pcanal avatar Apr 26 '24 19:04 pcanal

It shipped in Clang 17, which is the version that microsoft/STL currently requires. https://clang.llvm.org/cxx_dr_status.html documents this.

I believe that https://github.com/llvm/llvm-project/commit/00e2098bf49f0ed45b3b8c22894cd3ac9a530e0f implemented it, but there's a slight chance there were followup commits that I'm unaware of.

StephanTLavavej avatar Apr 27 '24 23:04 StephanTLavavej

OK, thanks a lot for the information! And since it's in Clang/Cling, let's add @vgvassilev in the loop...

bellenot avatar Apr 29 '24 07:04 bellenot

We probably should backport this commit. @hahnjo what do you think?

vgvassilev avatar Apr 29 '24 07:04 vgvassilev

Probably, but I'm a bit confused about the error message: Why would Cling output MSVC style error messages?

hahnjo avatar Apr 29 '24 07:04 hahnjo

I don't actually know what the G694476FC error code is (the only codes I recognize are MSVC Cnnnn, IntelliSense Ennnn, and Clang named warnings/errors), but the "Unexpected size" is coming from the STL. We have static_assert(false, "Unexpected size") in a template, and after the DR resolution, we expect that this should be activated only when that if constexpr branch is actually taken.

StephanTLavavej avatar Apr 30 '24 04:04 StephanTLavavej

I believe that llvm/llvm-project@00e2098 implemented it, but there's a slight chance there were followup commits that I'm unaware of.

Seems reasonable and applies cleanly to our LLVM 16 - I opened https://github.com/root-project/root/pull/15437 with the backport. I will wait for confirmation that this is all we need before syncing to our LLVM monorepo fork and eventually merging.

hahnjo avatar May 07 '24 12:05 hahnjo

@StephanTLavavej Thanks a lot for your help! Unfortunately, I can only try with VS 17.10 preview 6 (the only one available for now), and hence cannot try to reproduce the issue and see if the PR properly fixes it. I will try again as soon as v17.11 preview is available

bellenot avatar May 07 '24 13:05 bellenot

Fixed by https://github.com/root-project/root/pull/15437, backport to v6.32 via https://github.com/root-project/root/pull/15753

hahnjo avatar Jun 05 '24 06:06 hahnjo