[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam marked an inline comment as done.
zahiraam added a comment.

Aaron,

Please advise. 
Thanks.



================
Comment at: test/Sema/dllexport.c:70
+// const variables
+__declspec(dllexport) int const x = 3;
 
----------------
aaron.ballman wrote:
> Can you think of any redeclaration scenarios we should also test? e.g., weird cases like this:
> ```
> extern int const x;
> __declspec(dllexport) int const x = 3;
> ```
> which brings up an interesting question -- does MSVC truly treat it as though it were extern, or just only-kinda-sorta treat it like it was extern? e.g., can you do this:
> ```
> __declspec(dllexport) const int j;
> ```
> Regardless, having some codegen tests demonstrating that the variable has the correct semantics that we're emulating would be nice.
With MSVC:

ksh-3.2$ cat t2.cpp
__declspec(dllexport) const int j;
ksh-3.2$

ksh-3.2$ cl -c t2.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24210 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

t2.cpp
t2.cpp(1): error C2734: 'j': 'const' object must be initialized if not 'extern'

ksh-3.2$ cat t3.cpp
__declspec(dllexport) int const x = 3;
int main ()
{
  int y = x + 10;

  return y;
}

ksh-3.2$ cl -c t3.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24210 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

t3.cpp
ksh-3.2$ dumpbin /symbols t3.obj | grep External
**008 00000000 SECT3  notype ()    External     | main**
ksh-3.2$
ksh-3.2$ cat t4.cpp
extern int const x = 3;
int main ()
{
  int y = x + 10;

  return y;
}

ksh-3.2$ cl -c t4.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24210 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

t4.cpp
ksh-3.2$ dumpbin /symbols t4.obj | grep External
**008 00000000 SECT3  notype       External     | ?x@@3HB (int const x)
00B 00000000 SECT4  notype ()    External     | main**
ksh-3.2$

So we see that with dllexport, the ony symbol marked with "External" is main. With "extern" both main and x are marked as External.

With clang without the patch:
ksh-3.2$ clang -c t2.cpp
t2.cpp:1:33: error: default initialization of an object of const type 'const int'
__declspec(dllexport) const int j;
                                ^
                                  = 0
t2.cpp:1:33: error: 'j' must have external linkage when declared 'dllexport'
2 errors generated.
ksh-3.2$
ksh-3.2$ clang -c t3.cpp
t3.cpp:1:33: error: 'x' must have external linkage when declared 'dllexport'
__declspec(dllexport) int const x = 3;
                                ^
1 error generated.
ksh-3.2$ clang -c t4.cpp
ksh-3.2$ dumpbin /symbols t4.o | grep External
**00F 00000000 SECT1  notype ()    External     | main
010 00000000 SECT5  notype       External     | ?x@@3HB (int const x)**
ksh-3.2$

Clang with patch above at the right place (I am thinking in  Sema::AddInitializerToDecl):

ksh-3.2$ clang -c t3.cpp
ksh-3.2$ dumpbin /symbols t3.o | grep External
**00E 00000000 SECT1  notype ()    External     | main
00F 00000000 SECT5  notype       External     | ?x@@3HB (int const x)**
ksh-3.2$
ksh-3.2$ clang -c t4.cpp
ksh-3.2$ dumpbin /symbols t4.o | grep External
00C 00000000 SECT1  notype ()    External     | main
00D 00000000 SECT5  notype       External     | ?x@@3HB (int const x)
ksh-3.2$

Both MSVC and clang have the same behavior with t2.cpp.
With the patch clang will have the same beahvior than MSVC when extern and dllexport are used. That's not  quite what MSVC's behavior is!
What are your thoughts?
Thanks.




CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D45978/new/

https://reviews.llvm.org/D45978