Re: [PATCH] d: Committed fix multiple definition error when using mixins and interfaces.

2020-03-17 Thread Rainer Orth
Hi Iain,

> When generating thunks in the D front-end, they need not necesarily be
> in the same compilation unit as the module where the target function is
> declared.  When this happens, don't make the thunk public.  Later on
> expand_thunk will be called with force_gimple_thunk=true, so the thunk
> can never be made weak due to differing implementations between modules.
>
> Bootstrapped and tested on x86_64-linux-gnu, and committed to trunk.
>
> Regards
> Iain.
> ---
> gcc/d/ChangeLog:
>
>   PR d/92216
>   * decl.cc (make_thunk): Don't set TREE_PUBLIC on thunks if the target
>   function is external to the current compilation.
>
> gcc/testsuite/ChangeLog:
>
>   PR d/92216
>   * gdc.dg/imports/pr92216.d: New.
>   * gdc.dg/pr92216.d: New test.

this new testcase FAILs on 32-bit Solaris/SPARC and x86 (and, I suppose,
on every non-64-bit target):

+FAIL: gdc.dg/pr92216.d   -O0   scan-assembler 
_DT16_D7imports7pr922161B8__mixin24getSMFZPv[: \\t\\n]
+FAIL: gdc.dg/pr92216.d   -O0 -frelease   scan-assembler 
_DT16_D7imports7pr922161B8__mixin24getSMFZPv[: \\t\\n]
+FAIL: gdc.dg/pr92216.d   -O0 -frelease -g   scan-assembler 
_DT16_D7imports7pr922161B8__mixin24getSMFZPv[: \\t\\n]
+FAIL: gdc.dg/pr92216.d   -O0 -g   scan-assembler 
_DT16_D7imports7pr922161B8__mixin24getSMFZPv[: \\t\\n]

Same at -O[1-3s].  While the 64-bit version contains the expected

_DT16_D7imports7pr922161B8__mixin24getSMFZPv

the 32-bit one has

_DT8_D7imports7pr922161B8__mixin24getSMFZPv

I can't tell for certain if it's enough to allow for those two variants
or if more is needed.  Btw., I noticed that binutils 2.34 c++filt -s
dlang cannot demangle those symbols.  Is this expected?

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University


[PATCH] d: Committed fix multiple definition error when using mixins and interfaces.

2020-03-16 Thread Iain Buclaw via Gcc-patches
Hi,

When generating thunks in the D front-end, they need not necesarily be
in the same compilation unit as the module where the target function is
declared.  When this happens, don't make the thunk public.  Later on
expand_thunk will be called with force_gimple_thunk=true, so the thunk
can never be made weak due to differing implementations between modules.

Bootstrapped and tested on x86_64-linux-gnu, and committed to trunk.

Regards
Iain.
---
gcc/d/ChangeLog:

PR d/92216
* decl.cc (make_thunk): Don't set TREE_PUBLIC on thunks if the target
function is external to the current compilation.

gcc/testsuite/ChangeLog:

PR d/92216
* gdc.dg/imports/pr92216.d: New.
* gdc.dg/pr92216.d: New test.

---
 gcc/d/decl.cc  |  7 +--
 gcc/testsuite/gdc.dg/imports/pr92216.d | 22 ++
 gcc/testsuite/gdc.dg/pr92216.d | 13 +
 3 files changed, 40 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gdc.dg/imports/pr92216.d
 create mode 100644 gcc/testsuite/gdc.dg/pr92216.d

diff --git a/gcc/d/decl.cc b/gcc/d/decl.cc
index 3824060feb9..7afb1aa0292 100644
--- a/gcc/d/decl.cc
+++ b/gcc/d/decl.cc
@@ -1803,8 +1803,11 @@ make_thunk (FuncDeclaration *decl, int offset)
 
   DECL_CONTEXT (thunk) = d_decl_context (decl);
 
-  /* Thunks inherit the public access of the function they are targetting.  */
-  TREE_PUBLIC (thunk) = TREE_PUBLIC (function);
+  /* Thunks inherit the public access of the function they are targetting.
+ When the function is outside the current compilation unit however, then 
the
+ thunk must be kept private to not conflict.  */
+  TREE_PUBLIC (thunk) = TREE_PUBLIC (function) && !DECL_EXTERNAL (function);
+
   DECL_EXTERNAL (thunk) = 0;
 
   /* Thunks are always addressable.  */
diff --git a/gcc/testsuite/gdc.dg/imports/pr92216.d 
b/gcc/testsuite/gdc.dg/imports/pr92216.d
new file mode 100644
index 000..b8c71c03420
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/imports/pr92216.d
@@ -0,0 +1,22 @@
+module imports.pr92216;
+
+class B : I
+{
+protected override void getStruct(){}
+mixin A!();
+
+}
+
+mixin template A()
+{
+public void* getS()
+{
+return null;
+}
+}
+
+public interface I
+{
+public void* getS();
+protected void getStruct();
+}
diff --git a/gcc/testsuite/gdc.dg/pr92216.d b/gcc/testsuite/gdc.dg/pr92216.d
new file mode 100644
index 000..330604c9c89
--- /dev/null
+++ b/gcc/testsuite/gdc.dg/pr92216.d
@@ -0,0 +1,13 @@
+// https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92216
+// { dg-options "-I $srcdir/gdc.dg" }
+// { dg-do compile }
+// { dg-final { scan-assembler 
"_DT16_D7imports7pr922161B8__mixin24getSMFZPv\[: \t\n\]" } }
+// { dg-final { scan-assembler-not "(.globl|.global)\[ 
\]+_DT16_D7imports7pr922161B8__mixin24getSMFZPv" } }
+module pr92216;
+
+private import imports.pr92216;
+
+class C : B
+{
+protected override void getStruct() {}
+}
-- 
2.20.1