*From: *Hans Wennborg <h...@chromium.org>
*Date: *Wed, May 15, 2019 at 1:22 AM
> On Tue, May 14, 2019 at 6:35 PM Richard Smith <rich...@metafoo.co.uk>
> wrote:
> > Yep, I'm not at all surprised. Perhaps we should stop claiming to
> support this extension, given that it fundamentally violates assumptions
> made by clang (that linkage doesn't change after the first declaration).
> Presumably we instead used to miscompile the example you give above (and
> after the revert, miscompile it again now)?
>
> Maybe rnk has opinions too, but if we can get away with it, I think it
> would be nice if we could not claim to support this, maybe not error
> but dropping the static redeclaration of 'a' above with a warning. We
> could fix Chromium's code, maybe we can poke MS to fix the midl
> generated code, and maybe Firefox and others can work around the
> duplicate symbol error in PR41871 by passing "/client none" to
> midl.exe in the meantime.
>
> For my reduction above, I think we used to:
> - in clang 8, emit a with internal linkage (getting it right through
> luck somehow?)
> - after r359259, emit it with external linkage (causing PR41871,
> breaking Firefox)
> - after r360637, assert
>
The files are generated by the most up to date MIDL compiler, 8.0.1, and I
think it would be good to maintain compatibility with it until it a bug fix
is released.
It looks like MSVC gives an extern global turned-static static linkage in C
mode, and external linkage in C++ mode. It doesn't even agree with itself.
=/
$ cat t.cpp
typedef struct Foo { int x,y; } Foo;
extern const Foo my_global;
static const Foo my_global = { 1, 2 };
$ cl -nologo -TC -c t.cpp && dumpbin -symbols t.obj | grep my_global
t.cpp
008 00000000 SECT3 notype Static | my_global
$ cl -nologo -TP -c t.cpp && dumpbin -symbols t.obj | grep my_global
t.cpp
008 00000000 SECT3 notype External | ?my_global@@3UFoo@@B
(struct Foo const my_global)
So, I think in C++ mode, MSVC just ignores the second static.
We could probably re-land Richard's change (although it's quite a lot of
code...), but... drop the conflicting `static` on the floor
if hasLinkageBeenComputed() returns true. That would work around the
assert, right?
Those are the best ideas I have so far. :)
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
