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;
-- 
2.16.3
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;
-- 
2.16.3
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",
-- 
2.16.3
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;