win: Actually compile C++ as C++17, and fix __cplusplus macro [chromium/mini

Mark Mentovai (Gerrit)

unread,

Jan 7, 2022, 9:52:41 AM1/7/22

to Joshua Peraza, Roland Bock, crashp...@chromium.org

Attention is currently required from: Joshua Peraza.

View Change

To view, visit change 3372323. To unsubscribe, or for help writing mail filters, visit settings.

Joshua Peraza (Gerrit)

unread,

Jan 7, 2022, 12:32:29 PM1/7/22

to Mark Mentovai, Roland Bock, crashp...@chromium.org

Attention is currently required from: Mark Mentovai.

Patch set 2:Code-Review +1

View Change

To view, visit change 3372323. To unsubscribe, or for help writing mail filters, visit settings.

Mark Mentovai (Gerrit)

unread,

Jan 7, 2022, 1:42:56 PM1/7/22

to Joshua Peraza, Roland Bock, crashp...@chromium.org

Mark Mentovai submitted this change.

View Change

Approvals:
  Joshua Peraza: Looks good to me

win: Actually compile C++ as C++17, and fix __cplusplus macro

6562d2d0b2a8 intended to enable C++17, but an error in the
msvc_toolchain definition caused the tool("cxx") used for C++
compilation to use cflags_c (intended for C but not C++ compilations)
instead of the indended cflags_cc, which is where /std:c++17 was set.

Testing in Crashpad showed that /std:c++17 could cause compilation
failures in util/misc/no_cfi_icall.h. Changes in C++17 require
additional template specializations in that file, but only when using
C++17 or later. That file does contain the needed specializations for
C++17, making them conditionally available based on the value of the
__cplusplus macro. However, MSVC++ retains its traditional version of
__cplusplus, 199711l (indicating C++98/C++03), even under /std:c++17, at
least through MSVC++ 14/MSC 19. In order to obtain standards-conformant
behavior for this built-in macro, cl must be invoked with
/Zc:__cplusplus. When building with /std:c++17 /Zc:__cplusplus,
__cplusplus is defined as the expected 201703l, Crashpad’s
util/misc/no_cfi_icall.h enables the template specializations required
for C++17, and Crashpad is able to build successfully.

More information on /Zc:__cplusplus:
https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/,
https://docs.microsoft.com/en-us/cpp/build/reference/zc-cplusplus.

Change-Id: I9aa62f145a8c9f7a6b5c42e7a01039f356121782
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/mini_chromium/+/3372323
Reviewed-by: Joshua Peraza <jpe...@chromium.org>
---
M build/config/BUILD.gn
1 file changed, 39 insertions(+), 2 deletions(-)

diff --git a/build/config/BUILD.gn b/build/config/BUILD.gn
index d014759..ec7ec90 100644
--- a/build/config/BUILD.gn
+++ b/build/config/BUILD.gn
@@ -255,7 +255,10 @@
       "/wd4996",  # 'X' was declared deprecated.
     ]
 
-    cflags_cc = [ "/std:c++17" ]
+    cflags_cc = [
+      "/std:c++17",
+      "/Zc:__cplusplus",
+    ]
 
     ldflags += [ "/DEBUG" ]
 
@@ -667,7 +670,7 @@
       tool("cxx") {
         depfile = "{{output}}.d"
         pdbname = "{{target_out_dir}}/{{label_name}}_cc.pdb"
-        command = "ninja -t msvc -e $env -- $cxx /nologo /showIncludes {{defines}} {{include_dirs}} {{cflags}} {{cflags_c}}${extra_cflags}${extra_cflags_cc} /c {{source}} /Fo{{output}} /Fd\"$pdbname\""
+        command = "ninja -t msvc -e $env -- $cxx /nologo /showIncludes {{defines}} {{include_dirs}} {{cflags}} {{cflags_cc}}${extra_cflags}${extra_cflags_cc} /c {{source}} /Fo{{output}} /Fd\"$pdbname\""
         depsformat = "msvc"
         description = "CXX {{output}}"
         outputs =

To view, visit change 3372323. To unsubscribe, or for help writing mail filters, visit settings.

Reply all

Reply to author

Forward

win: Actually compile C++ as C++17, and fix __cplusplus macro [chromium/mini_chromium : main]

Mark Mentovai (Gerrit)

Joshua Peraza (Gerrit)

Mark Mentovai (Gerrit)