Envoy: quiche: fix compilation on Windows with MSVC

[sunjayBhatia]

Created this as an envoy issue instead of a chromium one as it is most relevant to our Windows port efforts and a chromium issue felt heavyweight, please let us know if we should instead make a chromium issue

Description:
Currently quiche does not compile with MSVC on Windows. In quic/core/quic_versions.h, MSVC does not evaluate this method as a valid constexpr b/c it does not infer the default constructor for ParsedQuicVersion. To fix this compilation issue on MSVC, we would like to add this patch to quiche upstream (and not have to patch quiche within envoy): https://github.com/greenhouse-org/envoy/commit/66eff7c1e12544a37792140039867481b9bc91f8#diff-becdb66bb80dae03d0d2daf48001a77c

Links:

• branch with change: https://github.com/greenhouse-org/envoy/tree/quiche-msvc-workaround

[sunjayBhatia]

cc @danzh2010 a quiche issue for your review

[danzh2010]

/assign danzh2010
Thanks for reporting this issue! I will added the fix in the next QUICHE update.

[danzh2010]

@sunjayBhatia Regarding your patch, we intentionally want to avoid the default constructor because there shouldn't be such use case. We didn't ran into this problem in chromium which also test windows build. Can you give us more error details and the compiler options you use?

[wrowe]

Hi @danzh2010 you can reproduce from envoy once PR 10293 is merged (it doesn't enable tests, but makes it possible to get everything compiled and then compile specific tests.) Our worktree has the workaround patch so it doesn't exhibit this behavior at all.

VS Buildtools chain with VC 16.4.5 (not fixed by 16.4.6), using stdc++14 compat, this test fails to compile;

ERROR: C:/_eb/external/com_googlesource_quiche/BUILD.bazel:3150:1: C++ compilation of rule '@com_googlesource_quiche//:quic_core_versions_lib' failed (Exit 2)
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche\quiche/quic/core/quic_versions.h(333): error C3615: constexpr function 'quic::SupportedVersions' cannot result in a constant expression
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche\quiche/quic/core/quic_versions.h(334): note: failure was caused by call of undefined function or one not declared 'constexpr'
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche\quiche/quic/core/quic_versions.h(334): note: see usage of 'std::array<quic::ParsedQuicVersion,8>::array'
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche/quiche/quic/core/quic_versions.cc(186): error C2131: expression did not evaluate to a constant
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche/quiche/quic/core/quic_versions.cc(186): note: function violates 'constexpr' rules or has errors
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche/quiche/quic/core/quic_versions.cc(186): note: see usage of 'quic::SupportedVersions'
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche/quiche/quic/core/quic_versions.cc(331): error C2131: expression did not evaluate to a constant
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche/quiche/quic/core/quic_versions.cc(331): note: function violates 'constexpr' rules or has errors
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche/quiche/quic/core/quic_versions.cc(331): note: see usage of 'quic::SupportedVersions'
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche/quiche/quic/core/quic_versions.cc(599): error C2131: expression did not evaluate to a constant
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche/quiche/quic/core/quic_versions.cc(599): note: function violates 'constexpr' rules or has errors
bazel-out/x64_windows-opt/bin/external/com_googlesource_quiche/quiche/quic/core/quic_versions.cc(599): note: see usage of 'quic::SupportedVersions'

[sunjayBhatia]

the specific test are in //test/extensions/quic_listeners/... (there may be others that indirectly require @com_googlesource_quiche//:quic_core_versions_lib however)

[wrowe]

@danzh2010 if you can have this addressed in the next few days, terrific! If not, we have the equivalent patch to envoy ready to PR with other //test/ fixes this coming week to continue our progress.

[danzh2010]

Adding a default constructor is not an option for QUICHE. As chromium windows build doesn't complain about this problem, I think probably some compiler options enabled in envoy caused this error. Is these a way for me to get all the build options for Windows CI?

[sunjayBhatia]

@danzh2010 is chromium building on Windows using llvm/clang, MSVC, etc.? We haven't had much luck with clang yet on Windows so we've stuck with MSVC, this might be another MSVC specific quirk

[danzh2010]

We use clang in chromium.

[wrowe]

We expected so, and are working to get Windows compiling on clang-cl. Of course the rules_foreign_cc and other quirks are posing a significant challenge, along with quirks with the LLVM build of clang-cl itself. (The MS flavor bundled in Visual Studio is unusable.)

We hope to announce progress on this in the coming weeks. In the meantime this fix is simply necessary, it isn't an attribute of the flags selected by Envoy, it is a brand new defect in the latest update of the source tree of Chromium. This was not an issue last autumn.

[danzh2010]

QUICHE has an upstream fix for this issue now. There will be dep update in this week to resolve this issue.