llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.03k stars 11.58k forks source link

MSVC: implement /Zc:externConstexpr- to make constexpr globals internal by default #35761

Open rnk opened 6 years ago

rnk commented 6 years ago
Bugzilla Link 36413
Version unspecified
OS Windows NT
CC @dpxf,@Ivan171,@nico

Extended Description

MSVC has a bug where some constexpr globals are given internal (static) linkage when the standard requires external linkage. Here is an example:

$ cat t.cpp extern int some_int; constexpr const int &int_ref = some_int; int useit() { return int_ref; }

$ cl -nologo -c t.cpp && dumpbin -symbols t.obj | grep int_ref t.cpp 009 00000000 SECT3 notype Static | ?int_ref@@3AEBHEB (int const & const int_ref)

$ clang-cl -c t.cpp && dumpbin -symbols t.obj | grep int_ref 00C 00000000 SECT4 notype External | ?int_ref@@3AEBHEB (int const & const int_ref)

See 'Static' vs. 'External'.

This lead to link errors when using certain parts of the 10.0.16299.0 Windows SDK in Chromium, originally reported here: https://crbug.com/780056 See the comments relating to MIDL_CONST_ID and windows.ui.viewmanagement.h.

MIDL appears to contain (or generate) this macro:

pragma push_macro("MIDL_CONST_ID")

if !defined(_MSC_VER) || (_MSC_VER >= 1910)

define MIDL_CONST_ID constexpr const

else

define MIDL_CONST_ID const __declspec(selectany)

endif

It seems that the authors expected 'constexpr' to imply __declspec(selectany), which is odd, because MSVC doesn't do that at all, it just makes the thing static, not comdat.

MSVC has a flag that disables this behavior, /Zc:externConstexpr, but I have to update VS before I can experiment with it.

nico commented 6 years ago

I don't think we should change our default, fwiw. It sounds like this is fixed in the latest Win10 SDK preview. Having an option to get the broken/cl-default-compat behavior would be nice though.