Support bitfields in Wodr machinery (PR lto/85405).

Message ID 7e3fc57b-c574-f7d4-e49e-6dba66bceff7@suse.cz
State New
Headers show
Series
  • Support bitfields in Wodr machinery (PR lto/85405).
Related show

Commit Message

Martin Liška April 17, 2018, 5:39 a.m.
Hi.

This is Honza's ODR warning patch that I've just tested. He approved that.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Martin

gcc/ChangeLog:

2018-04-16  Jan Hubicka  <jh@suse.cz>

	PR lto/85405
	* ipa-devirt.c (odr_types_equivalent_p): Handle bit fields.

gcc/testsuite/ChangeLog:

2018-04-16  Martin Liska  <mliska@suse.cz>

	PR lto/85405
	* g++.dg/lto/pr85405_0.C: New test.
	* g++.dg/lto/pr85405_1.C: New test.
---
 gcc/ipa-devirt.c                     | 11 +++++++++--
 gcc/testsuite/g++.dg/lto/pr85405_0.C | 18 ++++++++++++++++++
 gcc/testsuite/g++.dg/lto/pr85405_1.C |  9 +++++++++
 3 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405_1.C

Comments

Jakub Jelinek April 17, 2018, 6:58 a.m. | #1
On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
> +		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
> +		  {
> +		    warn_odr (t1, t2, f1, f2, warn, warned,
> +			      G_ ("one field is bitfield while other is not "));

I think all the G_ uses don't put a space in between G_ and (
Also, please avoid the trailing space in the message.

Do you diagnose if both are bit-fields, but with different bitcount, e.g. if
in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
and bool mbDisposed; with int mbDisposed : 7; ?

> +		    return false;
> +		  }
> +		else
> +		  gcc_assert (DECL_NONADDRESSABLE_P (f1)
> +			      == DECL_NONADDRESSABLE_P (f2));
>  	      }
>  
>  	    /* If one aggregate has more fields than the other, they
> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> new file mode 100644
> index 00000000000..1a41d81099c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> @@ -0,0 +1,18 @@
> +// { dg-lto-do link }
> +// { dg-lto-options {{-fPIC -shared -flto}} }
> +
> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
> +  int mnRefCnt;
> +  bool mbDisposed : 1;
> +  virtual ~VclReferenceBase();
> +};
> +class a;
> +class b {
> +  a &e;
> +  bool c();
> +};
> +class B {
> +  VclReferenceBase d;
> +};
> +class a : B {};
> +bool b::c() { return false; }
> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> new file mode 100644
> index 00000000000..78606185624
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> @@ -0,0 +1,9 @@
> +class VclReferenceBase {
> +  int mnRefCnt;
> +  bool mbDisposed;
> +
> +protected:
> +  virtual ~VclReferenceBase();
> +};
> +class : VclReferenceBase {
> +} a;
> 

	Jakub
Martin Liška April 17, 2018, 8:17 a.m. | #2
On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
> On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
>> +		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>> +		  {
>> +		    warn_odr (t1, t2, f1, f2, warn, warned,
>> +			      G_ ("one field is bitfield while other is not "));
> 
> I think all the G_ uses don't put a space in between G_ and (
> Also, please avoid the trailing space in the message.

Sure. I see other format violations, should I fix that in follow up patch:

gcc/c-family/c-warn.c:           ? G_ ("floating point overflow in expression %qE "
gcc/c-family/c-warn.c:           : G_ ("floating point overflow in expression of type %qT "
gcc/gimple-ssa-sprintf.c:                         ? G_ ("%<%.*s%> directive output between %wu and "
gcc/gimple-ssa-sprintf.c:                         : G_ ("%<%.*s%> directive output between %wu and "
gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"

?

> 
> Do you diagnose if both are bit-fields, but with different bitcount, e.g. if
> in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
> and bool mbDisposed; with int mbDisposed : 7; ?

Good point, I add a new test-case, done in patch.
Is it OK to install the patch?

Martin

> 
>> +		    return false;
>> +		  }
>> +		else
>> +		  gcc_assert (DECL_NONADDRESSABLE_P (f1)
>> +			      == DECL_NONADDRESSABLE_P (f2));
>>  	      }
>>  
>>  	    /* If one aggregate has more fields than the other, they
>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> new file mode 100644
>> index 00000000000..1a41d81099c
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> @@ -0,0 +1,18 @@
>> +// { dg-lto-do link }
>> +// { dg-lto-options {{-fPIC -shared -flto}} }
>> +
>> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>> +  int mnRefCnt;
>> +  bool mbDisposed : 1;
>> +  virtual ~VclReferenceBase();
>> +};
>> +class a;
>> +class b {
>> +  a &e;
>> +  bool c();
>> +};
>> +class B {
>> +  VclReferenceBase d;
>> +};
>> +class a : B {};
>> +bool b::c() { return false; }
>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> new file mode 100644
>> index 00000000000..78606185624
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> @@ -0,0 +1,9 @@
>> +class VclReferenceBase {
>> +  int mnRefCnt;
>> +  bool mbDisposed;
>> +
>> +protected:
>> +  virtual ~VclReferenceBase();
>> +};
>> +class : VclReferenceBase {
>> +} a;
>>
> 
> 	Jakub
>
From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 17 Apr 2018 10:11:07 +0200
Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).

gcc/ChangeLog:

2018-04-17  Martin Liska  <mliska@suse.cz>

	PR lto/85405
	* ipa-devirt.c (odr_types_equivalent_p):

gcc/testsuite/ChangeLog:

2018-04-17  Martin Liska  <mliska@suse.cz>

	PR lto/85405
	* g++.dg/lto/pr85405b_0.C: New test.
	* g++.dg/lto/pr85405b_1.C: New test.
---
 gcc/ipa-devirt.c                      |  2 +-
 gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++++++++++++++++++
 gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C

diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 85b8ef175f3..cc9b5e347e6 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
 		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
 		  {
 		    warn_odr (t1, t2, f1, f2, warn, warned,
-			      G_ ("one field is bitfield while other is not "));
+			      G_("one field is bitfield while other is not"));
 		    return false;
 		  }
 		else
diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
new file mode 100644
index 00000000000..a692abb7715
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
@@ -0,0 +1,18 @@
+// { dg-lto-do link }
+// { dg-lto-options {{-fPIC -shared -flto}} }
+
+class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
+  int mnRefCnt;
+  int mbDisposed : 3;
+  virtual ~VclReferenceBase();
+};
+class a;
+class b {
+  a &e;
+  bool c();
+};
+class B {
+  VclReferenceBase d;
+};
+class a : B {};
+bool b::c() { return false; }
diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_1.C b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
new file mode 100644
index 00000000000..fd98e631d56
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
@@ -0,0 +1,9 @@
+class VclReferenceBase {
+  int mnRefCnt;
+  int mbDisposed: 7; // { dg-lto-message "19: a field of same name but different type is defined in another translation unit" }
+
+protected:
+  virtual ~VclReferenceBase();
+};
+class : VclReferenceBase {
+} a;
Jan Hubicka April 17, 2018, 8:19 a.m. | #3
> On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
> > On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
> >> +		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
> >> +		  {
> >> +		    warn_odr (t1, t2, f1, f2, warn, warned,
> >> +			      G_ ("one field is bitfield while other is not "));
> > 
> > I think all the G_ uses don't put a space in between G_ and (
> > Also, please avoid the trailing space in the message.
> 
> Sure. I see other format violations, should I fix that in follow up patch:
> 
> gcc/c-family/c-warn.c:           ? G_ ("floating point overflow in expression %qE "
> gcc/c-family/c-warn.c:           : G_ ("floating point overflow in expression of type %qT "
> gcc/gimple-ssa-sprintf.c:                         ? G_ ("%<%.*s%> directive output between %wu and "
> gcc/gimple-ssa-sprintf.c:                         : G_ ("%<%.*s%> directive output between %wu and "
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"
> 
> ?
> 
> > 
> > Do you diagnose if both are bit-fields, but with different bitcount, e.g. if
> > in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
> > and bool mbDisposed; with int mbDisposed : 7; ?
> 
> Good point, I add a new test-case, done in patch.
> Is it OK to install the patch?
OK, thanks! (and sorry for the whitespace errors)
Honza
> 
> Martin
> 
> > 
> >> +		    return false;
> >> +		  }
> >> +		else
> >> +		  gcc_assert (DECL_NONADDRESSABLE_P (f1)
> >> +			      == DECL_NONADDRESSABLE_P (f2));
> >>  	      }
> >>  
> >>  	    /* If one aggregate has more fields than the other, they
> >> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> >> new file mode 100644
> >> index 00000000000..1a41d81099c
> >> --- /dev/null
> >> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
> >> @@ -0,0 +1,18 @@
> >> +// { dg-lto-do link }
> >> +// { dg-lto-options {{-fPIC -shared -flto}} }
> >> +
> >> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
> >> +  int mnRefCnt;
> >> +  bool mbDisposed : 1;
> >> +  virtual ~VclReferenceBase();
> >> +};
> >> +class a;
> >> +class b {
> >> +  a &e;
> >> +  bool c();
> >> +};
> >> +class B {
> >> +  VclReferenceBase d;
> >> +};
> >> +class a : B {};
> >> +bool b::c() { return false; }
> >> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> >> new file mode 100644
> >> index 00000000000..78606185624
> >> --- /dev/null
> >> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
> >> @@ -0,0 +1,9 @@
> >> +class VclReferenceBase {
> >> +  int mnRefCnt;
> >> +  bool mbDisposed;
> >> +
> >> +protected:
> >> +  virtual ~VclReferenceBase();
> >> +};
> >> +class : VclReferenceBase {
> >> +} a;
> >>
> > 
> > 	Jakub
> > 
> 

> From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
> From: marxin <mliska@suse.cz>
> Date: Tue, 17 Apr 2018 10:11:07 +0200
> Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).
> 
> gcc/ChangeLog:
> 
> 2018-04-17  Martin Liska  <mliska@suse.cz>
> 
> 	PR lto/85405
> 	* ipa-devirt.c (odr_types_equivalent_p):
> 
> gcc/testsuite/ChangeLog:
> 
> 2018-04-17  Martin Liska  <mliska@suse.cz>
> 
> 	PR lto/85405
> 	* g++.dg/lto/pr85405b_0.C: New test.
> 	* g++.dg/lto/pr85405b_1.C: New test.
> ---
>  gcc/ipa-devirt.c                      |  2 +-
>  gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++++++++++++++++++
>  gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +++++++++
>  3 files changed, 28 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C
> 
> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
> index 85b8ef175f3..cc9b5e347e6 100644
> --- a/gcc/ipa-devirt.c
> +++ b/gcc/ipa-devirt.c
> @@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
>  		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>  		  {
>  		    warn_odr (t1, t2, f1, f2, warn, warned,
> -			      G_ ("one field is bitfield while other is not "));
> +			      G_("one field is bitfield while other is not"));
>  		    return false;
>  		  }
>  		else
> diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
> new file mode 100644
> index 00000000000..a692abb7715
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
> @@ -0,0 +1,18 @@
> +// { dg-lto-do link }
> +// { dg-lto-options {{-fPIC -shared -flto}} }
> +
> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
> +  int mnRefCnt;
> +  int mbDisposed : 3;
> +  virtual ~VclReferenceBase();
> +};
> +class a;
> +class b {
> +  a &e;
> +  bool c();
> +};
> +class B {
> +  VclReferenceBase d;
> +};
> +class a : B {};
> +bool b::c() { return false; }
> diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_1.C b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
> new file mode 100644
> index 00000000000..fd98e631d56
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
> @@ -0,0 +1,9 @@
> +class VclReferenceBase {
> +  int mnRefCnt;
> +  int mbDisposed: 7; // { dg-lto-message "19: a field of same name but different type is defined in another translation unit" }
> +
> +protected:
> +  virtual ~VclReferenceBase();
> +};
> +class : VclReferenceBase {
> +} a;
> -- 
> 2.16.3
>
Jakub Jelinek April 17, 2018, 8:21 a.m. | #4
On Tue, Apr 17, 2018 at 10:17:15AM +0200, Martin Liška wrote:
> Sure. I see other format violations, should I fix that in follow up patch:
> 
> gcc/c-family/c-warn.c:           ? G_ ("floating point overflow in expression %qE "
> gcc/c-family/c-warn.c:           : G_ ("floating point overflow in expression of type %qT "
> gcc/gimple-ssa-sprintf.c:                         ? G_ ("%<%.*s%> directive output between %wu and "
> gcc/gimple-ssa-sprintf.c:                         : G_ ("%<%.*s%> directive output between %wu and "
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"

If you mean the space between G_ and (, sure, if you mean trailing
whitespace, note likely none of the above have trailing whitespace, at least
if they are followed by "something on another line.

> 2018-04-17  Martin Liska  <mliska@suse.cz>
> 
> 	PR lto/85405
> 	* ipa-devirt.c (odr_types_equivalent_p):

Please say what you've changed ;)

	Jakub
Martin Liška April 17, 2018, 8:28 a.m. | #5
On 04/17/2018 10:21 AM, Jakub Jelinek wrote:
> On Tue, Apr 17, 2018 at 10:17:15AM +0200, Martin Liška wrote:
>> Sure. I see other format violations, should I fix that in follow up patch:
>>
>> gcc/c-family/c-warn.c:           ? G_ ("floating point overflow in expression %qE "
>> gcc/c-family/c-warn.c:           : G_ ("floating point overflow in expression of type %qT "
>> gcc/gimple-ssa-sprintf.c:                         ? G_ ("%<%.*s%> directive output between %wu and "
>> gcc/gimple-ssa-sprintf.c:                         : G_ ("%<%.*s%> directive output between %wu and "
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"
> 
> If you mean the space between G_ and (, sure, if you mean trailing
> whitespace, note likely none of the above have trailing whitespace, at least
> if they are followed by "something on another line.

I'm sending patch candidate.

> 
>> 2018-04-17  Martin Liska  <mliska@suse.cz>
>>
>> 	PR lto/85405
>> 	* ipa-devirt.c (odr_types_equivalent_p):
> 
> Please say what you've changed ;)

Yes, done that in r259431.

M.

> 
> 	Jakub
>
From 143424c754415a20487e0dc615ac46cd934c8f78 Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 17 Apr 2018 10:11:07 +0200
Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).

gcc/ChangeLog:

2018-04-17  Martin Liska  <mliska@suse.cz>

	PR lto/85405
	* ipa-devirt.c (odr_types_equivalent_p): Remove trailing
	in message, remote space in between '_G' and '('.

gcc/testsuite/ChangeLog:

2018-04-17  Martin Liska  <mliska@suse.cz>

	PR lto/85405
	* g++.dg/lto/pr85405b_0.C: New test.
	* g++.dg/lto/pr85405b_1.C: New test.
---
 gcc/ipa-devirt.c                      |  2 +-
 gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++++++++++++++++++
 gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +++++++++
 3 files changed, 28 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
 create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C

diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index 85b8ef175f3..cc9b5e347e6 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
 		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
 		  {
 		    warn_odr (t1, t2, f1, f2, warn, warned,
-			      G_ ("one field is bitfield while other is not "));
+			      G_("one field is bitfield while other is not"));
 		    return false;
 		  }
 		else
diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
new file mode 100644
index 00000000000..a692abb7715
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
@@ -0,0 +1,18 @@
+// { dg-lto-do link }
+// { dg-lto-options {{-fPIC -shared -flto}} }
+
+class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
+  int mnRefCnt;
+  int mbDisposed : 3;
+  virtual ~VclReferenceBase();
+};
+class a;
+class b {
+  a &e;
+  bool c();
+};
+class B {
+  VclReferenceBase d;
+};
+class a : B {};
+bool b::c() { return false; }
diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_1.C b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
new file mode 100644
index 00000000000..fd98e631d56
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
@@ -0,0 +1,9 @@
+class VclReferenceBase {
+  int mnRefCnt;
+  int mbDisposed: 7; // { dg-lto-message "19: a field of same name but different type is defined in another translation unit" }
+
+protected:
+  virtual ~VclReferenceBase();
+};
+class : VclReferenceBase {
+} a;
Martin Liška April 17, 2018, 8:29 a.m. | #6
On 04/17/2018 10:28 AM, Martin Liška wrote:
> I'm sending patch candidate.

This one is the right one.

Martin
diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index d0d9c7894a8..2614eb58f14 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -98,10 +98,10 @@ overflow_warning (location_t loc, tree value, tree expr)
 
     case REAL_CST:
       warnfmt = (expr
-		 ? G_ ("floating point overflow in expression %qE "
-		       "of type %qT results in %qE")
-		 : G_ ("floating point overflow in expression of type %qT "
-		       "results in %qE"));
+		 ? G_("floating point overflow in expression %qE "
+		      "of type %qT results in %qE")
+		 : G_("floating point overflow in expression of type %qT "
+		      "results in %qE"));
       break;
 
     case FIXED_CST:
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 4ec58605ce8..ec5e7046f6e 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -2933,12 +2933,12 @@ format_directive (const sprintf_dom_walker::call_info &info,
       else
 	warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (),
 			  fmtres.range.min > target_int_max ()
-			  ? G_ ("%<%.*s%> directive output between %wu and "
-				"%wu bytes causes result to exceed "
-				"%<INT_MAX%>")
-			  : G_ ("%<%.*s%> directive output between %wu and "
-				"%wu bytes may cause result to exceed "
-				"%<INT_MAX%>"), dirlen,
+			  ? G_("%<%.*s%> directive output between %wu and "
+			       "%wu bytes causes result to exceed "
+			       "%<INT_MAX%>")
+			  : G_("%<%.*s%> directive output between %wu and "
+			       "%wu bytes may cause result to exceed "
+			       "%<INT_MAX%>"), dirlen,
 			  target_to_host (hostdir, sizeof hostdir, dir.beg),
 			  fmtres.range.min, fmtres.range.max);
     }
diff --git a/gcc/testsuite/gcc.dg/plugin/ggcplug.c b/gcc/testsuite/gcc.dg/plugin/ggcplug.c
index c4bc334868b..c186d119371 100644
--- a/gcc/testsuite/gcc.dg/plugin/ggcplug.c
+++ b/gcc/testsuite/gcc.dg/plugin/ggcplug.c
@@ -64,8 +64,8 @@ plugin_init (struct plugin_name_args *plugin_info,
       if (!strcmp (argv[i].key, "count-ggc-start"))
 	{
 	  if (argv[i].value)
-	    warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
-			    " ignored (superfluous '=%s')"),
+	    warning (0, G_("option '-fplugin-arg-%s-count-ggc-start=%s'"
+			   " ignored (superfluous '=%s')"),
 		     plugin_name, argv[i].value, argv[i].value);
 	  else
 	    register_callback ("ggcplug",
@@ -76,8 +76,8 @@ plugin_init (struct plugin_name_args *plugin_info,
       else if (!strcmp (argv[i].key, "count-ggc-end"))
 	{
 	  if (argv[i].value)
-	    warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
-			    " ignored (superfluous '=%s')"),
+	    warning (0, G_("option '-fplugin-arg-%s-count-ggc-end=%s'"
+			   " ignored (superfluous '=%s')"),
 		     plugin_name, argv[i].value, argv[i].value);
 	  else
 	    register_callback ("ggcplug",
@@ -88,8 +88,8 @@ plugin_init (struct plugin_name_args *plugin_info,
       else if (!strcmp (argv[i].key, "count-ggc-mark"))
 	{
 	  if (argv[i].value)
-	    warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
-			    " ignored (superfluous '=%s')"),
+	    warning (0, G_("option '-fplugin-arg-%s-count-ggc-mark=%s'"
+			   " ignored (superfluous '=%s')"),
 		     plugin_name, argv[i].value, argv[i].value);
 	  else
 	    register_callback ("ggcplug",
@@ -100,8 +100,8 @@ plugin_init (struct plugin_name_args *plugin_info,
       else if (!strcmp (argv[i].key, "test-extra-root"))
 	{
 	  if (argv[i].value)
-	    warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"
-			    " ignored (superfluous '=%s')"),
+	    warning (0, G_("option '-fplugin-arg-%s-test-extra-root=%s'"
+			   " ignored (superfluous '=%s')"),
 		     plugin_name, argv[i].value, argv[i].value);
 	  else
 	    register_callback ("ggcplug",
Jakub Jelinek April 17, 2018, 8:33 a.m. | #7
On Tue, Apr 17, 2018 at 10:29:35AM +0200, Martin Liška wrote:
> On 04/17/2018 10:28 AM, Martin Liška wrote:
> > I'm sending patch candidate.
> 
> This one is the right one.

Ok for stage1 with appropriate ChangeLog entries.

	Jakub
Martin Liška April 17, 2018, 12:03 p.m. | #8
On 04/17/2018 10:33 AM, Jakub Jelinek wrote:
> On Tue, Apr 17, 2018 at 10:29:35AM +0200, Martin Liška wrote:
>> On 04/17/2018 10:28 AM, Martin Liška wrote:
>>> I'm sending patch candidate.
>>
>> This one is the right one.
> 
> Ok for stage1 with appropriate ChangeLog entries.
> 
> 	Jakub
> 

Good, thanks. There's version I'll install once we flip into stage1 again.

Martin
From 93b53a0f55ebf4df7a96c620408af19546c7fc3a Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Tue, 17 Apr 2018 10:47:36 +0200
Subject: [PATCH] Fix GNU coding style for G_.

gcc/ChangeLog:

2018-04-17  Martin Liska  <mliska@suse.cz>

	* gimple-ssa-sprintf.c (format_directive): Do not use
	space in between 'G_' and '('.

gcc/c-family/ChangeLog:

2018-04-17  Martin Liska  <mliska@suse.cz>

	* c-warn.c (overflow_warning): Do not use
	space in between 'G_' and '('.

gcc/testsuite/ChangeLog:

2018-04-17  Martin Liska  <mliska@suse.cz>

	* gcc.dg/plugin/ggcplug.c (plugin_init): Do not use
	space in between 'G_' and '('.
---
 gcc/c-family/c-warn.c                 |  8 ++++----
 gcc/gimple-ssa-sprintf.c              | 12 ++++++------
 gcc/testsuite/gcc.dg/plugin/ggcplug.c | 16 ++++++++--------
 3 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/gcc/c-family/c-warn.c b/gcc/c-family/c-warn.c
index d0d9c7894a8..2614eb58f14 100644
--- a/gcc/c-family/c-warn.c
+++ b/gcc/c-family/c-warn.c
@@ -98,10 +98,10 @@ overflow_warning (location_t loc, tree value, tree expr)
 
     case REAL_CST:
       warnfmt = (expr
-		 ? G_ ("floating point overflow in expression %qE "
-		       "of type %qT results in %qE")
-		 : G_ ("floating point overflow in expression of type %qT "
-		       "results in %qE"));
+		 ? G_("floating point overflow in expression %qE "
+		      "of type %qT results in %qE")
+		 : G_("floating point overflow in expression of type %qT "
+		      "results in %qE"));
       break;
 
     case FIXED_CST:
diff --git a/gcc/gimple-ssa-sprintf.c b/gcc/gimple-ssa-sprintf.c
index 4ec58605ce8..ec5e7046f6e 100644
--- a/gcc/gimple-ssa-sprintf.c
+++ b/gcc/gimple-ssa-sprintf.c
@@ -2933,12 +2933,12 @@ format_directive (const sprintf_dom_walker::call_info &info,
       else
 	warned = fmtwarn (dirloc, argloc, NULL, info.warnopt (),
 			  fmtres.range.min > target_int_max ()
-			  ? G_ ("%<%.*s%> directive output between %wu and "
-				"%wu bytes causes result to exceed "
-				"%<INT_MAX%>")
-			  : G_ ("%<%.*s%> directive output between %wu and "
-				"%wu bytes may cause result to exceed "
-				"%<INT_MAX%>"), dirlen,
+			  ? G_("%<%.*s%> directive output between %wu and "
+			       "%wu bytes causes result to exceed "
+			       "%<INT_MAX%>")
+			  : G_("%<%.*s%> directive output between %wu and "
+			       "%wu bytes may cause result to exceed "
+			       "%<INT_MAX%>"), dirlen,
 			  target_to_host (hostdir, sizeof hostdir, dir.beg),
 			  fmtres.range.min, fmtres.range.max);
     }
diff --git a/gcc/testsuite/gcc.dg/plugin/ggcplug.c b/gcc/testsuite/gcc.dg/plugin/ggcplug.c
index c4bc334868b..c186d119371 100644
--- a/gcc/testsuite/gcc.dg/plugin/ggcplug.c
+++ b/gcc/testsuite/gcc.dg/plugin/ggcplug.c
@@ -64,8 +64,8 @@ plugin_init (struct plugin_name_args *plugin_info,
       if (!strcmp (argv[i].key, "count-ggc-start"))
 	{
 	  if (argv[i].value)
-	    warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
-			    " ignored (superfluous '=%s')"),
+	    warning (0, G_("option '-fplugin-arg-%s-count-ggc-start=%s'"
+			   " ignored (superfluous '=%s')"),
 		     plugin_name, argv[i].value, argv[i].value);
 	  else
 	    register_callback ("ggcplug",
@@ -76,8 +76,8 @@ plugin_init (struct plugin_name_args *plugin_info,
       else if (!strcmp (argv[i].key, "count-ggc-end"))
 	{
 	  if (argv[i].value)
-	    warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
-			    " ignored (superfluous '=%s')"),
+	    warning (0, G_("option '-fplugin-arg-%s-count-ggc-end=%s'"
+			   " ignored (superfluous '=%s')"),
 		     plugin_name, argv[i].value, argv[i].value);
 	  else
 	    register_callback ("ggcplug",
@@ -88,8 +88,8 @@ plugin_init (struct plugin_name_args *plugin_info,
       else if (!strcmp (argv[i].key, "count-ggc-mark"))
 	{
 	  if (argv[i].value)
-	    warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
-			    " ignored (superfluous '=%s')"),
+	    warning (0, G_("option '-fplugin-arg-%s-count-ggc-mark=%s'"
+			   " ignored (superfluous '=%s')"),
 		     plugin_name, argv[i].value, argv[i].value);
 	  else
 	    register_callback ("ggcplug",
@@ -100,8 +100,8 @@ plugin_init (struct plugin_name_args *plugin_info,
       else if (!strcmp (argv[i].key, "test-extra-root"))
 	{
 	  if (argv[i].value)
-	    warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"
-			    " ignored (superfluous '=%s')"),
+	    warning (0, G_("option '-fplugin-arg-%s-test-extra-root=%s'"
+			   " ignored (superfluous '=%s')"),
 		     plugin_name, argv[i].value, argv[i].value);
 	  else
 	    register_callback ("ggcplug",
Christophe Lyon April 18, 2018, 8:51 p.m. | #9
Hi,


On 17 April 2018 at 10:19, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
>> > On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
>> >> +          if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>> >> +            {
>> >> +              warn_odr (t1, t2, f1, f2, warn, warned,
>> >> +                        G_ ("one field is bitfield while other is not "));
>> >
>> > I think all the G_ uses don't put a space in between G_ and (
>> > Also, please avoid the trailing space in the message.
>>
>> Sure. I see other format violations, should I fix that in follow up patch:
>>
>> gcc/c-family/c-warn.c:           ? G_ ("floating point overflow in expression %qE "
>> gcc/c-family/c-warn.c:           : G_ ("floating point overflow in expression of type %qT "
>> gcc/gimple-ssa-sprintf.c:                         ? G_ ("%<%.*s%> directive output between %wu and "
>> gcc/gimple-ssa-sprintf.c:                         : G_ ("%<%.*s%> directive output between %wu and "
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"
>>
>> ?
>>
>> >
>> > Do you diagnose if both are bit-fields, but with different bitcount, e.g. if
>> > in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
>> > and bool mbDisposed; with int mbDisposed : 7; ?
>>
>> Good point, I add a new test-case, done in patch.
>> Is it OK to install the patch?
> OK, thanks! (and sorry for the whitespace errors)
> Honza
>>
>> Martin
>>
>> >
>> >> +              return false;
>> >> +            }
>> >> +          else
>> >> +            gcc_assert (DECL_NONADDRESSABLE_P (f1)
>> >> +                        == DECL_NONADDRESSABLE_P (f2));
>> >>          }
>> >>
>> >>        /* If one aggregate has more fields than the other, they
>> >> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> >> new file mode 100644
>> >> index 00000000000..1a41d81099c
>> >> --- /dev/null
>> >> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>> >> @@ -0,0 +1,18 @@
>> >> +// { dg-lto-do link }
>> >> +// { dg-lto-options {{-fPIC -shared -flto}} }
>> >> +
>> >> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>> >> +  int mnRefCnt;
>> >> +  bool mbDisposed : 1;
>> >> +  virtual ~VclReferenceBase();
>> >> +};
>> >> +class a;
>> >> +class b {
>> >> +  a &e;
>> >> +  bool c();
>> >> +};
>> >> +class B {
>> >> +  VclReferenceBase d;
>> >> +};
>> >> +class a : B {};
>> >> +bool b::c() { return false; }
>> >> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> >> new file mode 100644
>> >> index 00000000000..78606185624
>> >> --- /dev/null
>> >> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>> >> @@ -0,0 +1,9 @@
>> >> +class VclReferenceBase {
>> >> +  int mnRefCnt;
>> >> +  bool mbDisposed;
>> >> +
>> >> +protected:
>> >> +  virtual ~VclReferenceBase();
>> >> +};
>> >> +class : VclReferenceBase {
>> >> +} a;
>> >>
>> >
>> >     Jakub
>> >
>>
>
>> From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
>> From: marxin <mliska@suse.cz>
>> Date: Tue, 17 Apr 2018 10:11:07 +0200
>> Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).
>>
>> gcc/ChangeLog:
>>
>> 2018-04-17  Martin Liska  <mliska@suse.cz>
>>
>>       PR lto/85405
>>       * ipa-devirt.c (odr_types_equivalent_p):
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2018-04-17  Martin Liska  <mliska@suse.cz>
>>
>>       PR lto/85405
>>       * g++.dg/lto/pr85405b_0.C: New test.
>>       * g++.dg/lto/pr85405b_1.C: New test.

The new testcases require that -shared and -fpic work, which is not
the case on bare-metal targets
(eg arm-eabi, aarch64-elf).

The attached small patch adds
+// { dg-require-effective-target shared }
+// { dg-require-effective-target fpic }
so the tests are skipped on such targets.

OK for trunk?

Thanks,

Christophe

>> ---
>>  gcc/ipa-devirt.c                      |  2 +-
>>  gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++++++++++++++++++
>>  gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +++++++++
>>  3 files changed, 28 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C
>>
>> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
>> index 85b8ef175f3..cc9b5e347e6 100644
>> --- a/gcc/ipa-devirt.c
>> +++ b/gcc/ipa-devirt.c
>> @@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
>>               if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>>                 {
>>                   warn_odr (t1, t2, f1, f2, warn, warned,
>> -                           G_ ("one field is bitfield while other is not "));
>> +                           G_("one field is bitfield while other is not"));
>>                   return false;
>>                 }
>>               else
>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
>> new file mode 100644
>> index 00000000000..a692abb7715
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
>> @@ -0,0 +1,18 @@
>> +// { dg-lto-do link }
>> +// { dg-lto-options {{-fPIC -shared -flto}} }
>> +
>> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>> +  int mnRefCnt;
>> +  int mbDisposed : 3;
>> +  virtual ~VclReferenceBase();
>> +};
>> +class a;
>> +class b {
>> +  a &e;
>> +  bool c();
>> +};
>> +class B {
>> +  VclReferenceBase d;
>> +};
>> +class a : B {};
>> +bool b::c() { return false; }
>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_1.C b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
>> new file mode 100644
>> index 00000000000..fd98e631d56
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
>> @@ -0,0 +1,9 @@
>> +class VclReferenceBase {
>> +  int mnRefCnt;
>> +  int mbDisposed: 7; // { dg-lto-message "19: a field of same name but different type is defined in another translation unit" }
>> +
>> +protected:
>> +  virtual ~VclReferenceBase();
>> +};
>> +class : VclReferenceBase {
>> +} a;
>> --
>> 2.16.3
>>
>
gcc/testsuite/ChangeLog:

2018-04-18  Christophe Lyon  <christophe.lyon@linaro.org>

	* g++.dg/lto/pr85405_0.C: Require shared and fpic effective
	targets.
	* g++.dg/lto/pr85405b_0.C: Likewise.
diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C b/gcc/testsuite/g++.dg/lto/pr85405_0.C
index 1a41d81..1d46ea9 100644
--- a/gcc/testsuite/g++.dg/lto/pr85405_0.C
+++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
@@ -1,4 +1,6 @@
 // { dg-lto-do link }
+// { dg-require-effective-target shared }
+// { dg-require-effective-target fpic }
 // { dg-lto-options {{-fPIC -shared -flto}} }
 
 class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
index a692abb..a986a1f 100644
--- a/gcc/testsuite/g++.dg/lto/pr85405b_0.C
+++ b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
@@ -1,4 +1,6 @@
 // { dg-lto-do link }
+// { dg-require-effective-target shared }
+// { dg-require-effective-target fpic }
 // { dg-lto-options {{-fPIC -shared -flto}} }
 
 class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
Martin Liška April 19, 2018, 8:37 a.m. | #10
On 04/18/2018 10:51 PM, Christophe Lyon wrote:
> Hi,
> 
> 
> On 17 April 2018 at 10:19, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
>>>> On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
>>>>> +          if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>>>>> +            {
>>>>> +              warn_odr (t1, t2, f1, f2, warn, warned,
>>>>> +                        G_ ("one field is bitfield while other is not "));
>>>>
>>>> I think all the G_ uses don't put a space in between G_ and (
>>>> Also, please avoid the trailing space in the message.
>>>
>>> Sure. I see other format violations, should I fix that in follow up patch:
>>>
>>> gcc/c-family/c-warn.c:           ? G_ ("floating point overflow in expression %qE "
>>> gcc/c-family/c-warn.c:           : G_ ("floating point overflow in expression of type %qT "
>>> gcc/gimple-ssa-sprintf.c:                         ? G_ ("%<%.*s%> directive output between %wu and "
>>> gcc/gimple-ssa-sprintf.c:                         : G_ ("%<%.*s%> directive output between %wu and "
>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"
>>>
>>> ?
>>>
>>>>
>>>> Do you diagnose if both are bit-fields, but with different bitcount, e.g. if
>>>> in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
>>>> and bool mbDisposed; with int mbDisposed : 7; ?
>>>
>>> Good point, I add a new test-case, done in patch.
>>> Is it OK to install the patch?
>> OK, thanks! (and sorry for the whitespace errors)
>> Honza
>>>
>>> Martin
>>>
>>>>
>>>>> +              return false;
>>>>> +            }
>>>>> +          else
>>>>> +            gcc_assert (DECL_NONADDRESSABLE_P (f1)
>>>>> +                        == DECL_NONADDRESSABLE_P (f2));
>>>>>          }
>>>>>
>>>>>        /* If one aggregate has more fields than the other, they
>>>>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>>>>> new file mode 100644
>>>>> index 00000000000..1a41d81099c
>>>>> --- /dev/null
>>>>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>>>>> @@ -0,0 +1,18 @@
>>>>> +// { dg-lto-do link }
>>>>> +// { dg-lto-options {{-fPIC -shared -flto}} }
>>>>> +
>>>>> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>>>>> +  int mnRefCnt;
>>>>> +  bool mbDisposed : 1;
>>>>> +  virtual ~VclReferenceBase();
>>>>> +};
>>>>> +class a;
>>>>> +class b {
>>>>> +  a &e;
>>>>> +  bool c();
>>>>> +};
>>>>> +class B {
>>>>> +  VclReferenceBase d;
>>>>> +};
>>>>> +class a : B {};
>>>>> +bool b::c() { return false; }
>>>>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>>>>> new file mode 100644
>>>>> index 00000000000..78606185624
>>>>> --- /dev/null
>>>>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>>>>> @@ -0,0 +1,9 @@
>>>>> +class VclReferenceBase {
>>>>> +  int mnRefCnt;
>>>>> +  bool mbDisposed;
>>>>> +
>>>>> +protected:
>>>>> +  virtual ~VclReferenceBase();
>>>>> +};
>>>>> +class : VclReferenceBase {
>>>>> +} a;
>>>>>
>>>>
>>>>     Jakub
>>>>
>>>
>>
>>> From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
>>> From: marxin <mliska@suse.cz>
>>> Date: Tue, 17 Apr 2018 10:11:07 +0200
>>> Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).
>>>
>>> gcc/ChangeLog:
>>>
>>> 2018-04-17  Martin Liska  <mliska@suse.cz>
>>>
>>>       PR lto/85405
>>>       * ipa-devirt.c (odr_types_equivalent_p):
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 2018-04-17  Martin Liska  <mliska@suse.cz>
>>>
>>>       PR lto/85405
>>>       * g++.dg/lto/pr85405b_0.C: New test.
>>>       * g++.dg/lto/pr85405b_1.C: New test.
> 
> The new testcases require that -shared and -fpic work, which is not
> the case on bare-metal targets
> (eg arm-eabi, aarch64-elf).
> 
> The attached small patch adds
> +// { dg-require-effective-target shared }
> +// { dg-require-effective-target fpic }
> so the tests are skipped on such targets.
> 
> OK for trunk?

Thanks for the fix. It's quite obvious, please install it.

Martin

> 
> Thanks,
> 
> Christophe
> 
>>> ---
>>>  gcc/ipa-devirt.c                      |  2 +-
>>>  gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++++++++++++++++++
>>>  gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +++++++++
>>>  3 files changed, 28 insertions(+), 1 deletion(-)
>>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
>>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C
>>>
>>> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
>>> index 85b8ef175f3..cc9b5e347e6 100644
>>> --- a/gcc/ipa-devirt.c
>>> +++ b/gcc/ipa-devirt.c
>>> @@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
>>>               if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>>>                 {
>>>                   warn_odr (t1, t2, f1, f2, warn, warned,
>>> -                           G_ ("one field is bitfield while other is not "));
>>> +                           G_("one field is bitfield while other is not"));
>>>                   return false;
>>>                 }
>>>               else
>>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
>>> new file mode 100644
>>> index 00000000000..a692abb7715
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
>>> @@ -0,0 +1,18 @@
>>> +// { dg-lto-do link }
>>> +// { dg-lto-options {{-fPIC -shared -flto}} }
>>> +
>>> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>>> +  int mnRefCnt;
>>> +  int mbDisposed : 3;
>>> +  virtual ~VclReferenceBase();
>>> +};
>>> +class a;
>>> +class b {
>>> +  a &e;
>>> +  bool c();
>>> +};
>>> +class B {
>>> +  VclReferenceBase d;
>>> +};
>>> +class a : B {};
>>> +bool b::c() { return false; }
>>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_1.C b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
>>> new file mode 100644
>>> index 00000000000..fd98e631d56
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
>>> @@ -0,0 +1,9 @@
>>> +class VclReferenceBase {
>>> +  int mnRefCnt;
>>> +  int mbDisposed: 7; // { dg-lto-message "19: a field of same name but different type is defined in another translation unit" }
>>> +
>>> +protected:
>>> +  virtual ~VclReferenceBase();
>>> +};
>>> +class : VclReferenceBase {
>>> +} a;
>>> --
>>> 2.16.3
>>>
>>
Christophe Lyon April 19, 2018, 9 a.m. | #11
On 19 April 2018 at 10:37, Martin Liška <mliska@suse.cz> wrote:
> On 04/18/2018 10:51 PM, Christophe Lyon wrote:
>> Hi,
>>
>>
>> On 17 April 2018 at 10:19, Jan Hubicka <hubicka@ucw.cz> wrote:
>>>> On 04/17/2018 08:58 AM, Jakub Jelinek wrote:
>>>>> On Tue, Apr 17, 2018 at 07:39:20AM +0200, Martin Liška wrote:
>>>>>> +          if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>>>>>> +            {
>>>>>> +              warn_odr (t1, t2, f1, f2, warn, warned,
>>>>>> +                        G_ ("one field is bitfield while other is not "));
>>>>>
>>>>> I think all the G_ uses don't put a space in between G_ and (
>>>>> Also, please avoid the trailing space in the message.
>>>>
>>>> Sure. I see other format violations, should I fix that in follow up patch:
>>>>
>>>> gcc/c-family/c-warn.c:           ? G_ ("floating point overflow in expression %qE "
>>>> gcc/c-family/c-warn.c:           : G_ ("floating point overflow in expression of type %qT "
>>>> gcc/gimple-ssa-sprintf.c:                         ? G_ ("%<%.*s%> directive output between %wu and "
>>>> gcc/gimple-ssa-sprintf.c:                         : G_ ("%<%.*s%> directive output between %wu and "
>>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-start=%s'"
>>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-end=%s'"
>>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-count-ggc-mark=%s'"
>>>> gcc/testsuite/gcc.dg/plugin/ggcplug.c:      warning (0, G_ ("option '-fplugin-arg-%s-test-extra-root=%s'"
>>>>
>>>> ?
>>>>
>>>>>
>>>>> Do you diagnose if both are bit-fields, but with different bitcount, e.g. if
>>>>> in your testcase you replace bool mbDisposed : 1; with int mbDisposed : 3;
>>>>> and bool mbDisposed; with int mbDisposed : 7; ?
>>>>
>>>> Good point, I add a new test-case, done in patch.
>>>> Is it OK to install the patch?
>>> OK, thanks! (and sorry for the whitespace errors)
>>> Honza
>>>>
>>>> Martin
>>>>
>>>>>
>>>>>> +              return false;
>>>>>> +            }
>>>>>> +          else
>>>>>> +            gcc_assert (DECL_NONADDRESSABLE_P (f1)
>>>>>> +                        == DECL_NONADDRESSABLE_P (f2));
>>>>>>          }
>>>>>>
>>>>>>        /* If one aggregate has more fields than the other, they
>>>>>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>>>>>> new file mode 100644
>>>>>> index 00000000000..1a41d81099c
>>>>>> --- /dev/null
>>>>>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
>>>>>> @@ -0,0 +1,18 @@
>>>>>> +// { dg-lto-do link }
>>>>>> +// { dg-lto-options {{-fPIC -shared -flto}} }
>>>>>> +
>>>>>> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>>>>>> +  int mnRefCnt;
>>>>>> +  bool mbDisposed : 1;
>>>>>> +  virtual ~VclReferenceBase();
>>>>>> +};
>>>>>> +class a;
>>>>>> +class b {
>>>>>> +  a &e;
>>>>>> +  bool c();
>>>>>> +};
>>>>>> +class B {
>>>>>> +  VclReferenceBase d;
>>>>>> +};
>>>>>> +class a : B {};
>>>>>> +bool b::c() { return false; }
>>>>>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>>>>>> new file mode 100644
>>>>>> index 00000000000..78606185624
>>>>>> --- /dev/null
>>>>>> +++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
>>>>>> @@ -0,0 +1,9 @@
>>>>>> +class VclReferenceBase {
>>>>>> +  int mnRefCnt;
>>>>>> +  bool mbDisposed;
>>>>>> +
>>>>>> +protected:
>>>>>> +  virtual ~VclReferenceBase();
>>>>>> +};
>>>>>> +class : VclReferenceBase {
>>>>>> +} a;
>>>>>>
>>>>>
>>>>>     Jakub
>>>>>
>>>>
>>>
>>>> From 87380235f9b81bea4cf910e702192586c072ce93 Mon Sep 17 00:00:00 2001
>>>> From: marxin <mliska@suse.cz>
>>>> Date: Tue, 17 Apr 2018 10:11:07 +0200
>>>> Subject: [PATCH] Fix coding style and add a new test-case (PR lto/85405).
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2018-04-17  Martin Liska  <mliska@suse.cz>
>>>>
>>>>       PR lto/85405
>>>>       * ipa-devirt.c (odr_types_equivalent_p):
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>> 2018-04-17  Martin Liska  <mliska@suse.cz>
>>>>
>>>>       PR lto/85405
>>>>       * g++.dg/lto/pr85405b_0.C: New test.
>>>>       * g++.dg/lto/pr85405b_1.C: New test.
>>
>> The new testcases require that -shared and -fpic work, which is not
>> the case on bare-metal targets
>> (eg arm-eabi, aarch64-elf).
>>
>> The attached small patch adds
>> +// { dg-require-effective-target shared }
>> +// { dg-require-effective-target fpic }
>> so the tests are skipped on such targets.
>>
>> OK for trunk?
>
> Thanks for the fix. It's quite obvious, please install it.
>

Thanks, done.

I also patched gcc/testsuite/g++.dg/lto/pr84805_0.C in the same way.

Christophe

> Martin
>
>>
>> Thanks,
>>
>> Christophe
>>
>>>> ---
>>>>  gcc/ipa-devirt.c                      |  2 +-
>>>>  gcc/testsuite/g++.dg/lto/pr85405b_0.C | 18 ++++++++++++++++++
>>>>  gcc/testsuite/g++.dg/lto/pr85405b_1.C |  9 +++++++++
>>>>  3 files changed, 28 insertions(+), 1 deletion(-)
>>>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_0.C
>>>>  create mode 100644 gcc/testsuite/g++.dg/lto/pr85405b_1.C
>>>>
>>>> diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
>>>> index 85b8ef175f3..cc9b5e347e6 100644
>>>> --- a/gcc/ipa-devirt.c
>>>> +++ b/gcc/ipa-devirt.c
>>>> @@ -1599,7 +1599,7 @@ odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
>>>>               if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
>>>>                 {
>>>>                   warn_odr (t1, t2, f1, f2, warn, warned,
>>>> -                           G_ ("one field is bitfield while other is not "));
>>>> +                           G_("one field is bitfield while other is not"));
>>>>                   return false;
>>>>                 }
>>>>               else
>>>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_0.C b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
>>>> new file mode 100644
>>>> index 00000000000..a692abb7715
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/g++.dg/lto/pr85405b_0.C
>>>> @@ -0,0 +1,18 @@
>>>> +// { dg-lto-do link }
>>>> +// { dg-lto-options {{-fPIC -shared -flto}} }
>>>> +
>>>> +class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
>>>> +  int mnRefCnt;
>>>> +  int mbDisposed : 3;
>>>> +  virtual ~VclReferenceBase();
>>>> +};
>>>> +class a;
>>>> +class b {
>>>> +  a &e;
>>>> +  bool c();
>>>> +};
>>>> +class B {
>>>> +  VclReferenceBase d;
>>>> +};
>>>> +class a : B {};
>>>> +bool b::c() { return false; }
>>>> diff --git a/gcc/testsuite/g++.dg/lto/pr85405b_1.C b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
>>>> new file mode 100644
>>>> index 00000000000..fd98e631d56
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/g++.dg/lto/pr85405b_1.C
>>>> @@ -0,0 +1,9 @@
>>>> +class VclReferenceBase {
>>>> +  int mnRefCnt;
>>>> +  int mbDisposed: 7; // { dg-lto-message "19: a field of same name but different type is defined in another translation unit" }
>>>> +
>>>> +protected:
>>>> +  virtual ~VclReferenceBase();
>>>> +};
>>>> +class : VclReferenceBase {
>>>> +} a;
>>>> --
>>>> 2.16.3
>>>>
>>>
>

Patch

diff --git a/gcc/ipa-devirt.c b/gcc/ipa-devirt.c
index fa9380cce80..5da0f72d14f 100644
--- a/gcc/ipa-devirt.c
+++ b/gcc/ipa-devirt.c
@@ -1587,8 +1587,15 @@  odr_types_equivalent_p (tree t1, tree t2, bool warn, bool *warned,
 				 "in another translation unit"));
 		    return false;
 		  }
-		gcc_assert (DECL_NONADDRESSABLE_P (f1)
-			    == DECL_NONADDRESSABLE_P (f2));
+		if (DECL_BIT_FIELD (f1) != DECL_BIT_FIELD (f2))
+		  {
+		    warn_odr (t1, t2, f1, f2, warn, warned,
+			      G_ ("one field is bitfield while other is not "));
+		    return false;
+		  }
+		else
+		  gcc_assert (DECL_NONADDRESSABLE_P (f1)
+			      == DECL_NONADDRESSABLE_P (f2));
 	      }
 
 	    /* If one aggregate has more fields than the other, they
diff --git a/gcc/testsuite/g++.dg/lto/pr85405_0.C b/gcc/testsuite/g++.dg/lto/pr85405_0.C
new file mode 100644
index 00000000000..1a41d81099c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405_0.C
@@ -0,0 +1,18 @@ 
+// { dg-lto-do link }
+// { dg-lto-options {{-fPIC -shared -flto}} }
+
+class VclReferenceBase { // { dg-lto-warning "7: type 'struct VclReferenceBase' violates the C\\+\\+ One Definition Rule" }
+  int mnRefCnt;
+  bool mbDisposed : 1;
+  virtual ~VclReferenceBase();
+};
+class a;
+class b {
+  a &e;
+  bool c();
+};
+class B {
+  VclReferenceBase d;
+};
+class a : B {};
+bool b::c() { return false; }
diff --git a/gcc/testsuite/g++.dg/lto/pr85405_1.C b/gcc/testsuite/g++.dg/lto/pr85405_1.C
new file mode 100644
index 00000000000..78606185624
--- /dev/null
+++ b/gcc/testsuite/g++.dg/lto/pr85405_1.C
@@ -0,0 +1,9 @@ 
+class VclReferenceBase {
+  int mnRefCnt;
+  bool mbDisposed;
+
+protected:
+  virtual ~VclReferenceBase();
+};
+class : VclReferenceBase {
+} a;