Re: [PATCH] Improve DSE to handle redundant zero initializations.
Hello,
This should have the changes you wanted!
Thank you,
Matthew Beliveau
On Fri, Aug 16, 2019 at 12:30 PM Jeff Law wrote:
>
> On 8/13/19 9:09 AM, Matthew Beliveau wrote:
> > Hello,
> >
> > This should have the changes you all asked for! Let me know if I
> > missed anything!
> >
> > Thank you,
> > Matthew Beliveau
> >
> > On Tue, Aug 13, 2019 at 5:15 AM Richard Biener
> > wrote:
> >> On Tue, Aug 13, 2019 at 10:18 AM Richard Sandiford
> >> wrote:
> >>> Thanks for doing this.
> >>>
> >>> Matthew Beliveau writes:
> >>>> diff --git gcc/tree-ssa-dse.c gcc/tree-ssa-dse.c
> >>>> index 5b7c4fc6d1a..dcaeb8edbfe 100644
> >>>> --- gcc/tree-ssa-dse.c
> >>>> +++ gcc/tree-ssa-dse.c
> >>>> @@ -628,11 +628,8 @@ dse_optimize_redundant_stores (gimple *stmt)
> >>>>tree fndecl;
> >>>>if ((is_gimple_assign (use_stmt)
> >>>> && gimple_vdef (use_stmt)
> >>>> -&& ((gimple_assign_rhs_code (use_stmt) == CONSTRUCTOR
> >>>> - && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (use_stmt)) == 0
> >>>> - && !gimple_clobber_p (stmt))
> >>>> -|| (gimple_assign_rhs_code (use_stmt) == INTEGER_CST
> >>>> -&& integer_zerop (gimple_assign_rhs1 (use_stmt)
> >>>> +&& initializer_zerop (gimple_op (use_stmt, 1), NULL)
> >>>> +&& !gimple_clobber_p (stmt))
> >>>> || (gimple_call_builtin_p (use_stmt, BUILT_IN_NORMAL)
> >>>> && (fndecl = gimple_call_fndecl (use_stmt)) != NULL
> >>>> && (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_MEMSET
> >>>> @@ -1027,15 +1024,13 @@ dse_dom_walker::dse_optimize_stmt
> >>>> (gimple_stmt_iterator *gsi)
> >>>> {
> >>>>bool by_clobber_p = false;
> >>>>
> >>>> - /* First see if this store is a CONSTRUCTOR and if there
> >>>> - are subsequent CONSTRUCTOR stores which are totally
> >>>> - subsumed by this statement. If so remove the subsequent
> >>>> - CONSTRUCTOR store.
> >>>> + /* Check if this store initalizes zero, or some aggregate of
> >>>> zeros,
> >>>> + and check if there are subsequent stores which are subsumed by
> >>>> this
> >>>> + statement. If so, remove the subsequent store.
> >>>>
> >>>>This will tend to make fewer calls into memset with longer
> >>>>arguments. */
> >>>> - if (gimple_assign_rhs_code (stmt) == CONSTRUCTOR
> >>>> - && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (stmt)) == 0
> >>>> + if (initializer_zerop (gimple_op (stmt, 1), NULL)
> >>>> && !gimple_clobber_p (stmt))
> >>>> dse_optimize_redundant_stores (stmt);
> >>>>
> >>> In addition to Jeff's comment, the original choice of gimple_assign_rhs1
> >>> is the preferred way to write this (applies to both hunks).
> >> And the !gimple_clobber_p test is now redundant.
> >>
> >>> Richard
> >
> > DSE-2.patch
> >
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> >
> > 2019-08-13 Matthew Beliveau
> >
> > * tree-ssa-dse.c (dse_optimize_redundant_stores): Improved check to
> > catch more redundant zero initialization cases.
> > (dse_dom_walker::dse_optimize_stmt): Likewise.
> >
> > * gcc.dg/tree-ssa/redundant-assign-zero-1.c: New test.
> > * gcc.dg/tree-ssa/redundant-assign-zero-2.c: New test.
> >
> > diff --git gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
> > gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
> > new file mode 100644
> > index 000..b8d01d1644b
> > --- /dev/null
> > +++ gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
> > @@ -0,0 +1,13 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -fdump-tree-dse-details" } */
> > +
> > +void blah (char *);
> > +
> > +void bar ()
> > +{
> > + char a[256] = "";
> > + a[3] = 0;
> > + blah (a);
> > +}
> > +
> > +/* { dg-final { scan-tree-dump-times "Deleted redundant store" 1 "dse1"
Re: [PATCH] Improve DSE to handle redundant zero initializations.
Hello,
This should have the changes you all asked for! Let me know if I
missed anything!
Thank you,
Matthew Beliveau
On Tue, Aug 13, 2019 at 5:15 AM Richard Biener
wrote:
>
> On Tue, Aug 13, 2019 at 10:18 AM Richard Sandiford
> wrote:
> >
> > Thanks for doing this.
> >
> > Matthew Beliveau writes:
> > > diff --git gcc/tree-ssa-dse.c gcc/tree-ssa-dse.c
> > > index 5b7c4fc6d1a..dcaeb8edbfe 100644
> > > --- gcc/tree-ssa-dse.c
> > > +++ gcc/tree-ssa-dse.c
> > > @@ -628,11 +628,8 @@ dse_optimize_redundant_stores (gimple *stmt)
> > >tree fndecl;
> > >if ((is_gimple_assign (use_stmt)
> > > && gimple_vdef (use_stmt)
> > > -&& ((gimple_assign_rhs_code (use_stmt) == CONSTRUCTOR
> > > - && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (use_stmt)) == 0
> > > - && !gimple_clobber_p (stmt))
> > > -|| (gimple_assign_rhs_code (use_stmt) == INTEGER_CST
> > > -&& integer_zerop (gimple_assign_rhs1 (use_stmt)
> > > +&& initializer_zerop (gimple_op (use_stmt, 1), NULL)
> > > +&& !gimple_clobber_p (stmt))
> > > || (gimple_call_builtin_p (use_stmt, BUILT_IN_NORMAL)
> > > && (fndecl = gimple_call_fndecl (use_stmt)) != NULL
> > > && (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_MEMSET
> > > @@ -1027,15 +1024,13 @@ dse_dom_walker::dse_optimize_stmt
> > > (gimple_stmt_iterator *gsi)
> > > {
> > >bool by_clobber_p = false;
> > >
> > > - /* First see if this store is a CONSTRUCTOR and if there
> > > - are subsequent CONSTRUCTOR stores which are totally
> > > - subsumed by this statement. If so remove the subsequent
> > > - CONSTRUCTOR store.
> > > + /* Check if this store initalizes zero, or some aggregate of zeros,
> > > + and check if there are subsequent stores which are subsumed by this
> > > + statement. If so, remove the subsequent store.
> > >
> > >This will tend to make fewer calls into memset with longer
> > >arguments. */
> > > - if (gimple_assign_rhs_code (stmt) == CONSTRUCTOR
> > > - && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (stmt)) == 0
> > > + if (initializer_zerop (gimple_op (stmt, 1), NULL)
> > > && !gimple_clobber_p (stmt))
> > > dse_optimize_redundant_stores (stmt);
> > >
> >
> > In addition to Jeff's comment, the original choice of gimple_assign_rhs1
> > is the preferred way to write this (applies to both hunks).
>
> And the !gimple_clobber_p test is now redundant.
>
> > Richard
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-08-13 Matthew Beliveau
* tree-ssa-dse.c (dse_optimize_redundant_stores): Improved check to
catch more redundant zero initialization cases.
(dse_dom_walker::dse_optimize_stmt): Likewise.
* gcc.dg/tree-ssa/redundant-assign-zero-1.c: New test.
* gcc.dg/tree-ssa/redundant-assign-zero-2.c: New test.
diff --git gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
new file mode 100644
index 000..b8d01d1644b
--- /dev/null
+++ gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse-details" } */
+
+void blah (char *);
+
+void bar ()
+{
+ char a[256] = "";
+ a[3] = 0;
+ blah (a);
+}
+
+/* { dg-final { scan-tree-dump-times "Deleted redundant store" 1 "dse1"} } */
diff --git gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c
new file mode 100644
index 000..8cefa6f0cb7
--- /dev/null
+++ gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse-details" } */
+
+#include
+
+void blahd (double *);
+
+void fubar ()
+{
+ double d;
+ double *x =
+
+ memset (, 0 , sizeof d);
+ *x = 0.0;
+ blahd (x);
+}
+
+/* { dg-final { scan-tree-dump-times "Deleted redundant store" 1 "dse1"} } */
diff --git gcc/tree-ssa-dse.c gcc/tree-ssa-dse.c
index 5b7c4fc6d1a..14b66228e1e 100644
--- gcc/tree-ssa-dse.c
+++ gcc/tree-ssa-dse.c
@@ -628,11 +628,7 @@ dse_optimize_redundant_stores (gimple *stmt)
tree fndecl;
if ((is_gimple_assign (use_stmt)
&& gimple_vdef (use_stmt)
- && ((gimple_assign_rhs_code (use_stmt) == CONSTRUCTOR
- && CONSTRUCTOR_NELTS (gimple_assi
[PATCH] Improve DSE to handle redundant zero initializations.
This patch improves DSE to handle missing optimizations for zero
initializations.
Thank you,
Matthew Beliveau
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-08-12 Matthew Beliveau
PR c++/DSE.patch
* tree-ssa-dse.c (dse_optimize_redundant_stores): Improved check to
catch more redundant zero initalization cases.
(dse_dom_walker::dse_optimize_stmt): Improved check to catch more
redundant zero initialization cases.
* gcc.dg/tree-ssa/redundant-assign-zero-1.c: New test.
* gcc.dg/tree-ssa/redundant-assign-zero-2.c: New test.
diff --git gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
new file mode 100644
index 000..b8d01d1644b
--- /dev/null
+++ gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-1.c
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse-details" } */
+
+void blah (char *);
+
+void bar ()
+{
+ char a[256] = "";
+ a[3] = 0;
+ blah (a);
+}
+
+/* { dg-final { scan-tree-dump-times "Deleted redundant store" 1 "dse1"} } */
diff --git gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c
new file mode 100644
index 000..8cefa6f0cb7
--- /dev/null
+++ gcc/testsuite/gcc.dg/tree-ssa/redundant-assign-zero-2.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-dse-details" } */
+
+#include
+
+void blahd (double *);
+
+void fubar ()
+{
+ double d;
+ double *x =
+
+ memset (, 0 , sizeof d);
+ *x = 0.0;
+ blahd (x);
+}
+
+/* { dg-final { scan-tree-dump-times "Deleted redundant store" 1 "dse1"} } */
diff --git gcc/tree-ssa-dse.c gcc/tree-ssa-dse.c
index 5b7c4fc6d1a..dcaeb8edbfe 100644
--- gcc/tree-ssa-dse.c
+++ gcc/tree-ssa-dse.c
@@ -628,11 +628,8 @@ dse_optimize_redundant_stores (gimple *stmt)
tree fndecl;
if ((is_gimple_assign (use_stmt)
&& gimple_vdef (use_stmt)
- && ((gimple_assign_rhs_code (use_stmt) == CONSTRUCTOR
- && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (use_stmt)) == 0
- && !gimple_clobber_p (stmt))
- || (gimple_assign_rhs_code (use_stmt) == INTEGER_CST
- && integer_zerop (gimple_assign_rhs1 (use_stmt)
+ && initializer_zerop (gimple_op (use_stmt, 1), NULL)
+ && !gimple_clobber_p (stmt))
|| (gimple_call_builtin_p (use_stmt, BUILT_IN_NORMAL)
&& (fndecl = gimple_call_fndecl (use_stmt)) != NULL
&& (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_MEMSET
@@ -1027,15 +1024,13 @@ dse_dom_walker::dse_optimize_stmt (gimple_stmt_iterator *gsi)
{
bool by_clobber_p = false;
- /* First see if this store is a CONSTRUCTOR and if there
- are subsequent CONSTRUCTOR stores which are totally
- subsumed by this statement. If so remove the subsequent
- CONSTRUCTOR store.
+ /* Check if this store initalizes zero, or some aggregate of zeros,
+ and check if there are subsequent stores which are subsumed by this
+ statement. If so, remove the subsequent store.
This will tend to make fewer calls into memset with longer
arguments. */
- if (gimple_assign_rhs_code (stmt) == CONSTRUCTOR
- && CONSTRUCTOR_NELTS (gimple_assign_rhs1 (stmt)) == 0
+ if (initializer_zerop (gimple_op (stmt, 1), NULL)
&& !gimple_clobber_p (stmt))
dse_optimize_redundant_stores (stmt);
Re: [C++ PATCH] PR c++/90590 Suppress warning for enumeration value not handled in switch warning
ping
On Tue, Jul 23, 2019 at 10:27 AM Matthew Beliveau wrote:
>
> ping
>
> On Tue, Jul 16, 2019 at 8:40 AM Marek Polacek wrote:
> >
> > On Mon, Jul 15, 2019 at 09:47:26AM -0400, Matthew Beliveau wrote:
> > > Okay I kept the TYPE_MAIN_VARIANT and dropped the accidental new line!
> > > Hopefully this should be fine!
> >
> > CCing Joseph as this is c-family/ stuff.
> >
> > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > >
> > > 2019-07-12 Matthew Beliveau
> > >
> > > PR c++/90590
> > > * c-warn.c (c_do_switch_warnings): Suppress warning for enumerators
> > > with reserved names that are in a system header.
> > >
> > > * c-c++-common/pr90590-1.c: New test.
> > > * c-c++-common/pr90590-1.h: New test.
> > > * c-c++-common/pr90590-2.c: New test.
> > > * c-c++-common/pr90590-2.h: New test.
> > >
> > > diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
> > > index b5d09e761d7..51c54a283e5 100644
> > > --- gcc/c-family/c-warn.c
> > > +++ gcc/c-family/c-warn.c
> > > @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
> > > #include "gcc-rich-location.h"
> > > #include "gimplify.h"
> > > #include "c-family/c-indentation.h"
> > > +#include "c-family/c-spellcheck.h"
> > > #include "calls.h"
> > > #include "stor-layout.h"
> > >
> > > @@ -1628,6 +1629,15 @@ c_do_switch_warnings (splay_tree cases, location_t
> > > switch_location,
> > >if (cond && tree_int_cst_compare (cond, value))
> > > continue;
> > >
> > > + /* If the enumerator is defined in a system header and uses a
> > > reserved
> > > + name, then we continue to avoid throwing a warning. */
> > > + location_t loc = DECL_SOURCE_LOCATION
> > > + (TYPE_STUB_DECL (TYPE_MAIN_VARIANT (type)));
> > > + if (in_system_header_at (loc)
> > > + && name_reserved_for_implementation_p
> > > + (IDENTIFIER_POINTER (TREE_PURPOSE (chain
> > > + continue;
> > > +
> > >/* If there is a default_node, the only relevant option is
> > >Wswitch-enum. Otherwise, if both are enabled then we prefer
> > >to warn using -Wswitch because -Wswitch is enabled by -Wall
> > > diff --git gcc/testsuite/c-c++-common/pr90590-1.c
> > > gcc/testsuite/c-c++-common/pr90590-1.c
> > > new file mode 100644
> > > index 000..4e11debb7fa
> > > --- /dev/null
> > > +++ gcc/testsuite/c-c++-common/pr90590-1.c
> > > @@ -0,0 +1,15 @@
> > > +// PR c++/90590
> > > +// { dg-options -Wswitch }
> > > +#include "pr90590-1.h"
> > > +
> > > +void
> > > +g ()
> > > +{
> > > + enum E e = _A;
> > > + switch (e) // { dg-bogus "enumeration value '_C' not handled in
> > > switch" }
> > > +{
> > > +case _A:
> > > +case _B:
> > > + break;
> > > +}
> > > +}
> > > diff --git gcc/testsuite/c-c++-common/pr90590-1.h
> > > gcc/testsuite/c-c++-common/pr90590-1.h
> > > new file mode 100644
> > > index 000..22f1a7d5d52
> > > --- /dev/null
> > > +++ gcc/testsuite/c-c++-common/pr90590-1.h
> > > @@ -0,0 +1,2 @@
> > > +#pragma GCC system_header
> > > +enum E { _A, _B, _C };
> > > diff --git gcc/testsuite/c-c++-common/pr90590-2.c
> > > gcc/testsuite/c-c++-common/pr90590-2.c
> > > new file mode 100644
> > > index 000..23da97f9d74
> > > --- /dev/null
> > > +++ gcc/testsuite/c-c++-common/pr90590-2.c
> > > @@ -0,0 +1,11 @@
> > > +// PR c++/90590
> > > +// { dg-options -Wswitch }
> > > +
> > > +#include "pr90590-2.h"
> > > +
> > > +void
> > > +fn ()
> > > +{
> > > + switch (c.b) // { dg-bogus "enumeration value" }
> > > +;
> > > +}
> > > diff --git gcc/testsuite/c-c++-common/pr90590-2.h
> > > gcc/testsuite/c-c++-common/pr90590-2.h
> > > new file mode 100644
> > > index 000..e4f8635576f
> > > --- /dev/null
> > > +++ gcc/testsuite/c-c++-common/pr90590-2.h
> > > @@ -0,0 +1,4 @@
> > > +#pragma GCC system_header
> > > +struct {
> > > + enum { _A } b;
> > > +} c;
> >
> >
> > Marek
Re: [C++ PATCH] PR c++/90590 Suppress warning for enumeration value not handled in switch warning
ping
On Tue, Jul 16, 2019 at 8:40 AM Marek Polacek wrote:
>
> On Mon, Jul 15, 2019 at 09:47:26AM -0400, Matthew Beliveau wrote:
> > Okay I kept the TYPE_MAIN_VARIANT and dropped the accidental new line!
> > Hopefully this should be fine!
>
> CCing Joseph as this is c-family/ stuff.
>
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> >
> > 2019-07-12 Matthew Beliveau
> >
> > PR c++/90590
> > * c-warn.c (c_do_switch_warnings): Suppress warning for enumerators
> > with reserved names that are in a system header.
> >
> > * c-c++-common/pr90590-1.c: New test.
> > * c-c++-common/pr90590-1.h: New test.
> > * c-c++-common/pr90590-2.c: New test.
> > * c-c++-common/pr90590-2.h: New test.
> >
> > diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
> > index b5d09e761d7..51c54a283e5 100644
> > --- gcc/c-family/c-warn.c
> > +++ gcc/c-family/c-warn.c
> > @@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
> > #include "gcc-rich-location.h"
> > #include "gimplify.h"
> > #include "c-family/c-indentation.h"
> > +#include "c-family/c-spellcheck.h"
> > #include "calls.h"
> > #include "stor-layout.h"
> >
> > @@ -1628,6 +1629,15 @@ c_do_switch_warnings (splay_tree cases, location_t
> > switch_location,
> >if (cond && tree_int_cst_compare (cond, value))
> > continue;
> >
> > + /* If the enumerator is defined in a system header and uses a
> > reserved
> > + name, then we continue to avoid throwing a warning. */
> > + location_t loc = DECL_SOURCE_LOCATION
> > + (TYPE_STUB_DECL (TYPE_MAIN_VARIANT (type)));
> > + if (in_system_header_at (loc)
> > + && name_reserved_for_implementation_p
> > + (IDENTIFIER_POINTER (TREE_PURPOSE (chain
> > + continue;
> > +
> >/* If there is a default_node, the only relevant option is
> >Wswitch-enum. Otherwise, if both are enabled then we prefer
> >to warn using -Wswitch because -Wswitch is enabled by -Wall
> > diff --git gcc/testsuite/c-c++-common/pr90590-1.c
> > gcc/testsuite/c-c++-common/pr90590-1.c
> > new file mode 100644
> > index 000..4e11debb7fa
> > --- /dev/null
> > +++ gcc/testsuite/c-c++-common/pr90590-1.c
> > @@ -0,0 +1,15 @@
> > +// PR c++/90590
> > +// { dg-options -Wswitch }
> > +#include "pr90590-1.h"
> > +
> > +void
> > +g ()
> > +{
> > + enum E e = _A;
> > + switch (e) // { dg-bogus "enumeration value '_C' not handled in switch" }
> > +{
> > +case _A:
> > +case _B:
> > + break;
> > +}
> > +}
> > diff --git gcc/testsuite/c-c++-common/pr90590-1.h
> > gcc/testsuite/c-c++-common/pr90590-1.h
> > new file mode 100644
> > index 000..22f1a7d5d52
> > --- /dev/null
> > +++ gcc/testsuite/c-c++-common/pr90590-1.h
> > @@ -0,0 +1,2 @@
> > +#pragma GCC system_header
> > +enum E { _A, _B, _C };
> > diff --git gcc/testsuite/c-c++-common/pr90590-2.c
> > gcc/testsuite/c-c++-common/pr90590-2.c
> > new file mode 100644
> > index 000..23da97f9d74
> > --- /dev/null
> > +++ gcc/testsuite/c-c++-common/pr90590-2.c
> > @@ -0,0 +1,11 @@
> > +// PR c++/90590
> > +// { dg-options -Wswitch }
> > +
> > +#include "pr90590-2.h"
> > +
> > +void
> > +fn ()
> > +{
> > + switch (c.b) // { dg-bogus "enumeration value" }
> > +;
> > +}
> > diff --git gcc/testsuite/c-c++-common/pr90590-2.h
> > gcc/testsuite/c-c++-common/pr90590-2.h
> > new file mode 100644
> > index 000..e4f8635576f
> > --- /dev/null
> > +++ gcc/testsuite/c-c++-common/pr90590-2.h
> > @@ -0,0 +1,4 @@
> > +#pragma GCC system_header
> > +struct {
> > + enum { _A } b;
> > +} c;
>
>
> Marek
[PR rtl-optimization/91173] Backport to GCC 8 and 9
Tested on GCC 8 and 9.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-07-23 Matthew Beliveau
Backported from mainline
2019-07-16 Jeff Law
PR rtl-optimization/91173
* tree-ssa-address.c (addr_for_mem_ref): If the base is an
SSA_NAME with a constant value, fold its value into the offset
and clear the base before calling gen_addr_rtx.
* g++.dg/pr91173.C: New test.
diff --git gcc/testsuite/g++.dg/pr91173.C gcc/testsuite/g++.dg/pr91173.C
new file mode 100644
index 000..b8fb41ba0cd
--- /dev/null
+++ gcc/testsuite/g++.dg/pr91173.C
@@ -0,0 +1,45 @@
+class a {
+ int b;
+ void *c;
+
+public:
+ bool aa();
+ int () {
+if (aa()) {
+ void *d(c);
+ return static_cast(d)[b];
+}
+return *(int *)0;
+ }
+};
+typedef enum {E} e;
+class f : public a {
+ int g;
+
+public:
+ int ac() {
+if (g)
+ return 1;
+return ac();
+ }
+};
+int *ad;
+struct h {
+ static int ae(e, int *m) {
+f ag;
+int *ah;
+while (!0) {
+ ad = ();
+ ah = ad + ag.ac();
+ while (ad < ah)
+*m = *ad++;
+}
+ }
+};
+template
+void i(int *, int *, int, int *, e n, int *o) {
+ h::ae(n, o);
+}
+int aq, ar, as, at, au;
+void aw() { i(, , as, , (e)0, ); }
+
diff --git gcc/tree-ssa-address.c gcc/tree-ssa-address.c
index 1c17e935914..2e5d87734d6 100644
--- gcc/tree-ssa-address.c
+++ gcc/tree-ssa-address.c
@@ -259,6 +259,20 @@ addr_for_mem_ref (struct mem_address *addr, addr_space_t as,
? expand_expr (addr->index, NULL_RTX, pointer_mode, EXPAND_NORMAL)
: NULL_RTX);
+ /* addr->base could be an SSA_NAME that was set to a constant value. The
+ call to expand_expr may expose that constant. If so, fold the value
+ into OFF and clear BSE. Otherwise we may later try to pull a mode from
+ BSE to generate a REG, which won't work with constants because they
+ are modeless. */
+ if (bse && GET_CODE (bse) == CONST_INT)
+{
+ if (off)
+ off = simplify_gen_binary (PLUS, pointer_mode, bse, off);
+ else
+ off = bse;
+ gcc_assert (GET_CODE (off) == CONST_INT);
+ bse = NULL_RTX;
+}
gen_addr_rtx (pointer_mode, sym, bse, idx, st, off, , NULL, NULL);
if (pointer_mode != address_mode)
address = convert_memory_address (address_mode, address);
Re: [C++ PATCH] PR c++/90590 Suppress warning for enumeration value not handled in switch warning
Okay I kept the TYPE_MAIN_VARIANT and dropped the accidental new line!
Hopefully this should be fine!
On Fri, Jul 12, 2019 at 6:49 PM Marek Polacek wrote:
>
> On Fri, Jul 12, 2019 at 02:38:59PM -0400, Marek Polacek wrote:
> > On Fri, Jul 12, 2019 at 02:34:37PM -0400, Matthew Beliveau wrote:
> > > @@ -1628,6 +1629,16 @@ c_do_switch_warnings (splay_tree cases, location_t
> > > switch_location,
> > >if (cond && tree_int_cst_compare (cond, value))
> > > continue;
> > >
> > > + /* If the enumerator is defined in a system header and uses a
> > > reserved
> > > +name, then we continue to avoid throwing a warning. */
> > > + location_t loc = DECL_SOURCE_LOCATION
> > > + (TYPE_STUB_DECL (TYPE_MAIN_VARIANT (type)));
> >
> > As I mentioned before, I wonder if we can get away without the
> > TYPE_MAIN_VARIANT here.
>
> Ah, without TYPE_MAIN_VARIANT there's this ICEs:
>
> /opt/notnfs/polacek/gcc/libstdc++-v3/src/c++11/debug.cc: In function ‘void
> {anonymous}::print_field({anonymous}::PrintContext&, const _Parameter&, const
> char*)’:
> /opt/notnfs/polacek/gcc/libstdc++-v3/src/c++11/debug.cc:791:5: internal
> compiler error: Segmentation fault
> 791 | }
> | ^
> 0xf31def crash_signal
> /opt/notnfs/polacek/gcc/gcc/toplev.c:326
> 0xaaa579 contains_struct_check(tree_node*, tree_node_structure_enum, char
> const*, int, char const*)
> /opt/notnfs/polacek/gcc/gcc/tree.h:3330
> 0xaaa579 c_do_switch_warnings(splay_tree_s*, unsigned int, tree_node*,
> tree_node*, bool)
> /opt/notnfs/polacek/gcc/gcc/c-family/c-warn.c:1634
> 0x8ab61f pop_switch()
> /opt/notnfs/polacek/gcc/gcc/cp/decl.c:3567
> 0x9e0af4 finish_switch_stmt(tree_node*)
> /opt/notnfs/polacek/gcc/gcc/cp/semantics.c:1229
> 0x95764d cp_parser_selection_statement
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:11964
> 0x95764d cp_parser_statement
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:11186
> 0x958770 cp_parser_statement_seq_opt
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:11667
> 0x958847 cp_parser_compound_statement
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:11621
> 0x971990 cp_parser_function_body
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:22651
> 0x971990 cp_parser_ctor_initializer_opt_and_function_body
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:22702
> 0x972216 cp_parser_function_definition_after_declarator
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:28016
> 0x972fae cp_parser_function_definition_from_specifiers_and_declarator
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:27932
> 0x972fae cp_parser_init_declarator
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:20288
> 0x954ecf cp_parser_simple_declaration
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:13546
> 0x9798a2 cp_parser_declaration
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:13243
> 0x97a46c cp_parser_declaration_seq_opt
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:13119
> 0x97a46c cp_parser_namespace_body
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:19335
> 0x97a46c cp_parser_namespace_definition
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:19313
> 0x9799b3 cp_parser_declaration
> /opt/notnfs/polacek/gcc/gcc/cp/parser.c:13223
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See <https://gcc.gnu.org/bugs/> for instructions.
>
> So seems like we need it after all. Sorry about that.
>
> Marek
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-07-12 Matthew Beliveau
PR c++/90590
* c-warn.c (c_do_switch_warnings): Suppress warning for enumerators
with reserved names that are in a system header.
* c-c++-common/pr90590-1.c: New test.
* c-c++-common/pr90590-1.h: New test.
* c-c++-common/pr90590-2.c: New test.
* c-c++-common/pr90590-2.h: New test.
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index b5d09e761d7..51c54a283e5 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
#include "gcc-rich-location.h"
#include "gimplify.h"
#include "c-family/c-indentation.h"
+#include "c-family/c-spellcheck.h"
#include "calls.h"
#include "stor-layout.h"
@@ -1628,6 +1629,15 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
if (cond && tree_int_cst_compare (cond, value))
continue;
+ /* If the enumerator is defined in a system header and uses a reserved
+ name, then we continue to avoid throwing a warning. */
+ location
Re: [C++ PATCH] PR c++/90590 Suppress warning for enumeration value not handled in switch warning
Hi,
This should fix the concerns you had! Now the function only gets the
location from the type, and then only look for the name if the type's
location is in the header.
On Tue, Jul 9, 2019 at 5:21 PM Jason Merrill wrote:
>
> On 7/9/19 11:18 AM, Matthew Beliveau wrote:
> > This patch suppresses the warning: "enumeration value not handled in
> > switch", for enumerators that are defined in system headers and use
> > reserved names.
>
> > + if (decl == NULL_TREE)
> > + decl = lookup_name (TREE_PURPOSE (chain));
>
> This seems likely to find an unrelated declaration. If we have a name
> without a decl, I think it would be better to just look at that name
> rather than try to find the corresponding decl. For location, we can
> use the location of the type.
>
> Jason
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-07-12 Matthew Beliveau
PR c++/90590
* c-warn.c (c_do_switch_warnings): Suppress warning for enumerators
with reserved names that are in a system header.
* c-c++-common/pr90590-1.c: New test.
* c-c++-common/pr90590-1.h: New test.
* c-c++-common/pr90590-2.c: New test.
* c-c++-common/pr90590-2.h: New test.
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index b5d09e761d7..e120ad96f4b 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
#include "gcc-rich-location.h"
#include "gimplify.h"
#include "c-family/c-indentation.h"
+#include "c-family/c-spellcheck.h"
#include "calls.h"
#include "stor-layout.h"
@@ -1628,6 +1629,16 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
if (cond && tree_int_cst_compare (cond, value))
continue;
+ /* If the enumerator is defined in a system header and uses a reserved
+ name, then we continue to avoid throwing a warning. */
+ location_t loc = DECL_SOURCE_LOCATION
+ (TYPE_STUB_DECL (TYPE_MAIN_VARIANT (type)));
+ if (in_system_header_at (loc)
+ && name_reserved_for_implementation_p
+ (IDENTIFIER_POINTER (TREE_PURPOSE (chain
+ continue;
+
+
/* If there is a default_node, the only relevant option is
Wswitch-enum. Otherwise, if both are enabled then we prefer
to warn using -Wswitch because -Wswitch is enabled by -Wall
diff --git gcc/testsuite/c-c++-common/pr90590-1.c gcc/testsuite/c-c++-common/pr90590-1.c
new file mode 100644
index 000..4e11debb7fa
--- /dev/null
+++ gcc/testsuite/c-c++-common/pr90590-1.c
@@ -0,0 +1,15 @@
+// PR c++/90590
+// { dg-options -Wswitch }
+#include "pr90590-1.h"
+
+void
+g ()
+{
+ enum E e = _A;
+ switch (e) // { dg-bogus "enumeration value '_C' not handled in switch" }
+{
+case _A:
+case _B:
+ break;
+}
+}
diff --git gcc/testsuite/c-c++-common/pr90590-1.h gcc/testsuite/c-c++-common/pr90590-1.h
new file mode 100644
index 000..22f1a7d5d52
--- /dev/null
+++ gcc/testsuite/c-c++-common/pr90590-1.h
@@ -0,0 +1,2 @@
+#pragma GCC system_header
+enum E { _A, _B, _C };
diff --git gcc/testsuite/c-c++-common/pr90590-2.c gcc/testsuite/c-c++-common/pr90590-2.c
new file mode 100644
index 000..23da97f9d74
--- /dev/null
+++ gcc/testsuite/c-c++-common/pr90590-2.c
@@ -0,0 +1,11 @@
+// PR c++/90590
+// { dg-options -Wswitch }
+
+#include "pr90590-2.h"
+
+void
+fn ()
+{
+ switch (c.b) // { dg-bogus "enumeration value" }
+;
+}
diff --git gcc/testsuite/c-c++-common/pr90590-2.h gcc/testsuite/c-c++-common/pr90590-2.h
new file mode 100644
index 000..e4f8635576f
--- /dev/null
+++ gcc/testsuite/c-c++-common/pr90590-2.h
@@ -0,0 +1,4 @@
+#pragma GCC system_header
+struct {
+ enum { _A } b;
+} c;
[C++ PATCH] PR c++/90590 Suppress warning for enumeration value not handled in switch warning
This patch suppresses the warning: "enumeration value not handled in
switch", for enumerators that are defined in system headers and use
reserved names.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-07-08 Matthew Beliveau
PR c++/90590
* c-warn.c (c_do_switch_warnings): Suppress warning for enumerators
with reserved names that are in a system header.
* c-c++-common/pr90590-1.c: New test.
* c-c++-common/pr90590-1.h: New test.
* c-c++-common/pr90590-2.c: New test.
* c-c++-common/pr90590-2.h: New test.
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index b5d09e761d7..56ad23dd29c 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
#include "gcc-rich-location.h"
#include "gimplify.h"
#include "c-family/c-indentation.h"
+#include "c-family/c-spellcheck.h"
#include "calls.h"
#include "stor-layout.h"
@@ -1592,8 +1593,12 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
for (chain = TYPE_VALUES (type); chain; chain = TREE_CHAIN (chain))
{
tree value = TREE_VALUE (chain);
+ tree decl = NULL_TREE;
if (TREE_CODE (value) == CONST_DECL)
- value = DECL_INITIAL (value);
+ {
+ decl = value;
+ value = DECL_INITIAL (value);
+ }
node = splay_tree_lookup (cases, (splay_tree_key) value);
if (node)
{
@@ -1628,6 +1633,19 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
if (cond && tree_int_cst_compare (cond, value))
continue;
+ /* If the enumerator is defined in a system header and uses a reserved
+ name, then we continue to avoid throwing a warning. */
+ if (decl == NULL_TREE)
+ decl = lookup_name (TREE_PURPOSE (chain));
+ if (decl && TREE_CODE (decl) == CONST_DECL)
+ {
+ const char *name = IDENTIFIER_POINTER (DECL_NAME (decl));
+ location_t loc = DECL_SOURCE_LOCATION (decl);
+ if (in_system_header_at (loc)
+ && name_reserved_for_implementation_p (name))
+ continue;
+ }
+
/* If there is a default_node, the only relevant option is
Wswitch-enum. Otherwise, if both are enabled then we prefer
to warn using -Wswitch because -Wswitch is enabled by -Wall
diff --git gcc/testsuite/c-c++-common/pr90590-1.c gcc/testsuite/c-c++-common/pr90590-1.c
new file mode 100644
index 000..4997a3082d5
--- /dev/null
+++ gcc/testsuite/c-c++-common/pr90590-1.c
@@ -0,0 +1,15 @@
+// PR c++/90590
+// { dg-options -Wswitch }
+#include "pr90590-1.h"
+
+void
+g ()
+{
+ enum E e = _A;
+ switch (e) // { dg-bogus "enumeration value '_C' not handled in switch" }
+{
+case _A:
+case _B:
+ break;
+}
+}
diff --git gcc/testsuite/c-c++-common/pr90590-1.h gcc/testsuite/c-c++-common/pr90590-1.h
new file mode 100644
index 000..22f1a7d5d52
--- /dev/null
+++ gcc/testsuite/c-c++-common/pr90590-1.h
@@ -0,0 +1,2 @@
+#pragma GCC system_header
+enum E { _A, _B, _C };
diff --git gcc/testsuite/c-c++-common/pr90590-2.c gcc/testsuite/c-c++-common/pr90590-2.c
new file mode 100644
index 000..8aa65cf0afd
--- /dev/null
+++ gcc/testsuite/c-c++-common/pr90590-2.c
@@ -0,0 +1,8 @@
+#include "pr90590-2.h"
+
+void
+fn ()
+{
+ switch (c.b) // { dg-bogus "enumeration value" }
+;
+}
diff --git gcc/testsuite/c-c++-common/pr90590-2.h gcc/testsuite/c-c++-common/pr90590-2.h
new file mode 100644
index 000..e4f8635576f
--- /dev/null
+++ gcc/testsuite/c-c++-common/pr90590-2.h
@@ -0,0 +1,4 @@
+#pragma GCC system_header
+struct {
+ enum { _A } b;
+} c;
Re: [C++ PATCH] PR c++/90875 - added -Wswitch-outside-range option.
Sorry, the last version had some problems.
On Thu, Jun 20, 2019 at 4:26 PM Matthew Beliveau wrote:
>
> Hopefully fixed!
>
> On Thu, Jun 20, 2019 at 2:42 PM Marek Polacek wrote:
> >
> > On Thu, Jun 20, 2019 at 09:33:18AM -0400, Matthew Beliveau wrote:
> > > Sorry for the last update, I guess I didn't notice the other changes,
> > > oops!
> > >
> > > This should have all the changes. Let me know if anything went wrong!
> > >
> > > Thanks,
> > > Matthew Beliveau
> > >
> > > On Tue, Jun 18, 2019 at 1:38 PM Marek Polacek wrote:
> > > >
> > > > On Tue, Jun 18, 2019 at 01:17:10PM -0400, Matthew Beliveau wrote:
> > > > > Hello,
> > > > >
> > > > > This patch should change the formatting, and move the test files into
> > > > > the appropriate directory!
> > > >
> > > > It doesn't address my other comments, though, so please send a new
> > > > version
> > > > with that fixed.
> > > >
> > > > Marek
> >
> > > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> > >
> > > 2019-06-20 Matthew Beliveau
> > >
> > > PR c++/90875 - added -Wswitch-outside-range option
> > > * doc/invoke.texi (Wswitch-outside-range): Document.
> > >
> > > * c-warn.c (c_do_switch_warnings): Implemented new
> > > Wswitch-outside-range
> > > warning option.
> > >
> > > * c.opt (Wswitch-outside-range): Added new option.
> > >
> > > * c-c++-common/Wswitch-outside-range-1.C: New test.
> > > * c-c++-common/Wswitch-outside-range-2.C: New test.
> > > * c-c++-common/Wswitch-outside-range-3.C: New test.
> > > * c-c++-common/Wswitch-outside-range-4.C: New test.
> > >
> > > diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
> > > index 5941c10cddb..743099c75ca 100644
> > > --- gcc/c-family/c-warn.c
> > > +++ gcc/c-family/c-warn.c
> > > @@ -1460,8 +1460,9 @@ c_do_switch_warnings (splay_tree cases, location_t
> > > switch_location,
> > > min_value) >= 0)
> > > {
> > > location_t loc = EXPR_LOCATION ((tree) node->value);
> > > - warning_at (loc, 0, "lower value in case label range"
> > > - " less than minimum value for type");
> > > + warning_at (loc, OPT_Wswitch_outside_range,
> > > + "lower value in case label range less than minimum
> > > value"
> > > + " for type");
> > > CASE_LOW ((tree) node->value) = convert (TREE_TYPE (cond),
> > > min_value);
> > > node->key = (splay_tree_key) CASE_LOW ((tree) node->value);
> > > @@ -1474,8 +1475,8 @@ c_do_switch_warnings (splay_tree cases, location_t
> > > switch_location,
> > > if (node == NULL || !node->key)
> > > break;
> > > location_t loc = EXPR_LOCATION ((tree) node->value);
> > > - warning_at (loc, 0, "case label value is less than minimum "
> > > - "value for type");
> > > + warning_at (loc, OPT_Wswitch_outside_range, "case label value
> > > is"
> > > + " less than minimum value for type");
> > > splay_tree_remove (cases, node->key);
> > > }
> > > while (1);
> > > @@ -1491,8 +1492,8 @@ c_do_switch_warnings (splay_tree cases, location_t
> > > switch_location,
> > > max_value) > 0)
> > > {
> > > location_t loc = EXPR_LOCATION ((tree) node->value);
> > > - warning_at (loc, 0, "upper value in case label range"
> > > - " exceeds maximum value for type");
> > > + warning_at (loc, OPT_Wswitch_outside_range, "upper value in case"
> > > + " label range exceeds maximum value for
> > > type");
> > > CASE_HIGH ((tree) node->value)
> > > = convert (TREE_TYPE (cond), max_value);
> > > outside_range_p = true;
> >
> > The formatting is still wrong here...
> >
> >
Re: [C++ PATCH] PR c++/90875 - added -Wswitch-outside-range option.
Hopefully fixed!
On Thu, Jun 20, 2019 at 2:42 PM Marek Polacek wrote:
>
> On Thu, Jun 20, 2019 at 09:33:18AM -0400, Matthew Beliveau wrote:
> > Sorry for the last update, I guess I didn't notice the other changes, oops!
> >
> > This should have all the changes. Let me know if anything went wrong!
> >
> > Thanks,
> > Matthew Beliveau
> >
> > On Tue, Jun 18, 2019 at 1:38 PM Marek Polacek wrote:
> > >
> > > On Tue, Jun 18, 2019 at 01:17:10PM -0400, Matthew Beliveau wrote:
> > > > Hello,
> > > >
> > > > This patch should change the formatting, and move the test files into
> > > > the appropriate directory!
> > >
> > > It doesn't address my other comments, though, so please send a new version
> > > with that fixed.
> > >
> > > Marek
>
> > Bootstrapped/regtested on x86_64-linux, ok for trunk?
> >
> > 2019-06-20 Matthew Beliveau
> >
> > PR c++/90875 - added -Wswitch-outside-range option
> > * doc/invoke.texi (Wswitch-outside-range): Document.
> >
> > * c-warn.c (c_do_switch_warnings): Implemented new
> > Wswitch-outside-range
> > warning option.
> >
> > * c.opt (Wswitch-outside-range): Added new option.
> >
> > * c-c++-common/Wswitch-outside-range-1.C: New test.
> > * c-c++-common/Wswitch-outside-range-2.C: New test.
> > * c-c++-common/Wswitch-outside-range-3.C: New test.
> > * c-c++-common/Wswitch-outside-range-4.C: New test.
> >
> > diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
> > index 5941c10cddb..743099c75ca 100644
> > --- gcc/c-family/c-warn.c
> > +++ gcc/c-family/c-warn.c
> > @@ -1460,8 +1460,9 @@ c_do_switch_warnings (splay_tree cases, location_t
> > switch_location,
> > min_value) >= 0)
> > {
> > location_t loc = EXPR_LOCATION ((tree) node->value);
> > - warning_at (loc, 0, "lower value in case label range"
> > - " less than minimum value for type");
> > + warning_at (loc, OPT_Wswitch_outside_range,
> > + "lower value in case label range less than minimum value"
> > + " for type");
> > CASE_LOW ((tree) node->value) = convert (TREE_TYPE (cond),
> > min_value);
> > node->key = (splay_tree_key) CASE_LOW ((tree) node->value);
> > @@ -1474,8 +1475,8 @@ c_do_switch_warnings (splay_tree cases, location_t
> > switch_location,
> > if (node == NULL || !node->key)
> > break;
> > location_t loc = EXPR_LOCATION ((tree) node->value);
> > - warning_at (loc, 0, "case label value is less than minimum "
> > - "value for type");
> > + warning_at (loc, OPT_Wswitch_outside_range, "case label value
> > is"
> > + " less than minimum value for type");
> > splay_tree_remove (cases, node->key);
> > }
> > while (1);
> > @@ -1491,8 +1492,8 @@ c_do_switch_warnings (splay_tree cases, location_t
> > switch_location,
> > max_value) > 0)
> > {
> > location_t loc = EXPR_LOCATION ((tree) node->value);
> > - warning_at (loc, 0, "upper value in case label range"
> > - " exceeds maximum value for type");
> > + warning_at (loc, OPT_Wswitch_outside_range, "upper value in case"
> > + " label range exceeds maximum value for type");
> > CASE_HIGH ((tree) node->value)
> > = convert (TREE_TYPE (cond), max_value);
> > outside_range_p = true;
>
> The formatting is still wrong here...
>
> > @@ -1503,7 +1504,7 @@ c_do_switch_warnings (splay_tree cases, location_t
> > switch_location,
> >!= NULL)
> > {
> > location_t loc = EXPR_LOCATION ((tree) node->value);
> > - warning_at (loc, 0,
> > + warning_at (loc, OPT_Wswitch_outside_range,
> > "case label value exceeds maximum value for type");
> > splay_tree_remove (cases, node->key);
> > outside_range_p = true;
>
> ...but is correct here. So make the other cases above like this one.
>
> >
Re: [C++ PATCH] PR c++/90875 - added -Wswitch-outside-range option.
Sorry for the last update, I guess I didn't notice the other changes, oops!
This should have all the changes. Let me know if anything went wrong!
Thanks,
Matthew Beliveau
On Tue, Jun 18, 2019 at 1:38 PM Marek Polacek wrote:
>
> On Tue, Jun 18, 2019 at 01:17:10PM -0400, Matthew Beliveau wrote:
> > Hello,
> >
> > This patch should change the formatting, and move the test files into
> > the appropriate directory!
>
> It doesn't address my other comments, though, so please send a new version
> with that fixed.
>
> Marek
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-06-20 Matthew Beliveau
PR c++/90875 - added -Wswitch-outside-range option
* doc/invoke.texi (Wswitch-outside-range): Document.
* c-warn.c (c_do_switch_warnings): Implemented new Wswitch-outside-range
warning option.
* c.opt (Wswitch-outside-range): Added new option.
* c-c++-common/Wswitch-outside-range-1.C: New test.
* c-c++-common/Wswitch-outside-range-2.C: New test.
* c-c++-common/Wswitch-outside-range-3.C: New test.
* c-c++-common/Wswitch-outside-range-4.C: New test.
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index 5941c10cddb..743099c75ca 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -1460,8 +1460,9 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
min_value) >= 0)
{
location_t loc = EXPR_LOCATION ((tree) node->value);
- warning_at (loc, 0, "lower value in case label range"
- " less than minimum value for type");
+ warning_at (loc, OPT_Wswitch_outside_range,
+ "lower value in case label range less than minimum value"
+ " for type");
CASE_LOW ((tree) node->value) = convert (TREE_TYPE (cond),
min_value);
node->key = (splay_tree_key) CASE_LOW ((tree) node->value);
@@ -1474,8 +1475,8 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
if (node == NULL || !node->key)
break;
location_t loc = EXPR_LOCATION ((tree) node->value);
- warning_at (loc, 0, "case label value is less than minimum "
- "value for type");
+ warning_at (loc, OPT_Wswitch_outside_range, "case label value is"
+ " less than minimum value for type");
splay_tree_remove (cases, node->key);
}
while (1);
@@ -1491,8 +1492,8 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
max_value) > 0)
{
location_t loc = EXPR_LOCATION ((tree) node->value);
- warning_at (loc, 0, "upper value in case label range"
- " exceeds maximum value for type");
+ warning_at (loc, OPT_Wswitch_outside_range, "upper value in case"
+ " label range exceeds maximum value for type");
CASE_HIGH ((tree) node->value)
= convert (TREE_TYPE (cond), max_value);
outside_range_p = true;
@@ -1503,7 +1504,7 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
!= NULL)
{
location_t loc = EXPR_LOCATION ((tree) node->value);
- warning_at (loc, 0,
+ warning_at (loc, OPT_Wswitch_outside_range,
"case label value exceeds maximum value for type");
splay_tree_remove (cases, node->key);
outside_range_p = true;
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 572cf186262..a4cf3bd623d 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -819,6 +819,10 @@ Wswitch-bool
C ObjC C++ ObjC++ Var(warn_switch_bool) Warning Init(1)
Warn about switches with boolean controlling expression.
+Wswitch-outside-range
+C ObjC C++ ObjC++ Var(warn_switch_outside_range) Warning Init(1)
+Warn about switch values that are outside of the switch's type range.
+
Wtemplates
C++ ObjC++ Var(warn_templates) Warning
Warn on primary template declaration.
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index eaef4cd63d2..210535cb84a 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -5390,6 +5390,13 @@ switch ((int) (a == 4))
@end smallexample
This warning is enabled by default for C and C++ programs.
+@item -Wswitch-outside-range
+@opindex Wswitch-outside-range
+@opindex Wno-switch-outside-range
+Warn whenever a @code{switch} state has a value that is outside of its
+respective type range. This warning is enabled by default for
+C and C++ progarams.
+
@item -Wswitch-unreachable
@opindex Wswitch-unreachable
@opindex Wno-switch-unreachable
diff --git gcc/testsuite/c-c++-common/Wswitch-outside-range-1.C gcc/testsuite/c-c++-common/Wswitch-outside-range-1.C
new file mode 100644
index 000..29e56f3ba2d
--- /dev/null
+++ gcc/testsuite/c-c++-common/Wswitch-outside-range-1.C
@@ -0,0 +1,8 @@
+// PR c++/90875
+
+void f(char c)
+{
+ switch (c)
+case 300: // { dg-warning "case label value exceeds maximum value for type" }
+case -300:; // { dg-warning &quo
Re: [C++ PATCH] PR c++/90875 - added -Wswitch-outside-range option.
Hello,
This patch should change the formatting, and move the test files into
the appropriate directory!
Thank you
On Mon, Jun 17, 2019 at 1:45 PM Marek Polacek wrote:
>
> Thanks for the patch.
>
> On Mon, Jun 17, 2019 at 01:26:37PM -0400, Matthew Beliveau wrote:
> > --- gcc/c-family/c.opt
> > +++ gcc/c-family/c.opt
> > @@ -819,6 +819,10 @@ Wswitch-bool
> > C ObjC C++ ObjC++ Var(warn_switch_bool) Warning Init(1)
> > Warn about switches with boolean controlling expression.
> >
> > +Wswitch-outside-range
> > +C++ ObjC++ Var(warn_switch_outside_range) Warning Init(1)
>
> This warning is not C++- specific; it also applies to C and Obj-C.
>
> > +Warn about switch values that are outside of their type's range.
>
> This is slightly imprecise -- the values are outside of the type
> of the controlling expression of the switch. So I'd say
> "that are outside of the switch's type range" or so.
>
> > diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
> > index bf9da0f0a6e..c23496b2668 100644
> > --- gcc/doc/invoke.texi
> > +++ gcc/doc/invoke.texi
> > @@ -5390,6 +5390,12 @@ switch ((int) (a == 4))
> > @end smallexample
> > This warning is enabled by default for C and C++ programs.
> >
> > +@item -Wswitch-outside-range
> > +@opindex Wswitch-outside-range
> > +@opindex Wno-switch-outside-range
> > +Warn whenever a @code{switch} state has a value that is outside of it's
>
> "its"
>
> > +respective type range.
> > +
>
> This should also say
> "This warning is enabled by default for C and C++ programs."
>
> > diff --git gcc/testsuite/g++.dg/warn/Wswitch-outside-range-1.C
> > gcc/testsuite/g++.dg/warn/Wswitch-outside-range-1.C
> > new file mode 100644
> > index 000..29e56f3ba2d
> > --- /dev/null
> > +++ gcc/testsuite/g++.dg/warn/Wswitch-outside-range-1.C
> > @@ -0,0 +1,8 @@
> > +// PR c++/90875
> > +
> > +void f(char c)
> > +{
> > + switch (c)
> > +case 300: // { dg-warning "case label value exceeds maximum value for
> > type" }
> > +case -300:; // { dg-warning "case label value is less than minimum
> > value for type" }
> > +}
> > diff --git gcc/testsuite/g++.dg/warn/Wswitch-outside-range-2.C
> > gcc/testsuite/g++.dg/warn/Wswitch-outside-range-2.C
> > new file mode 100644
> > index 000..20cc019b209
> > --- /dev/null
> > +++ gcc/testsuite/g++.dg/warn/Wswitch-outside-range-2.C
> > @@ -0,0 +1,9 @@
> > +// PR c++/90875
> > +// { dg-options -Wno-switch-outside-range }
> > +
> > +void f(char c)
> > +{
> > + switch (c)
> > +case 300: //{ dg-bogus "case label value is less than minimum value
> > for type" }
> > +case -300:; // { dg-bogus "case label value is less than minimum value
> > for type" }
> > +}
> > diff --git gcc/testsuite/g++.dg/warn/Wswitch-outside-range-3.C
> > gcc/testsuite/g++.dg/warn/Wswitch-outside-range-3.C
> > new file mode 100644
> > index 000..baf15561af0
> > --- /dev/null
> > +++ gcc/testsuite/g++.dg/warn/Wswitch-outside-range-3.C
> > @@ -0,0 +1,9 @@
> > +// PR c++/90875
> > +// { dg-options -Wno-pedantic }
> > +
> > +void f(char c)
> > +{
> > + switch (c)
> > +
> > +case -300 ... 300:; // { dg-warning "lower value in case label range
> > less than minimum value for type|upper value in case label range exceeds
> > maximum value for type" }
> > +}
> > diff --git gcc/testsuite/g++.dg/warn/Wswitch-outside-range-4.C
> > gcc/testsuite/g++.dg/warn/Wswitch-outside-range-4.C
> > new file mode 100644
> > index 000..d9bd756dc50
> > --- /dev/null
> > +++ gcc/testsuite/g++.dg/warn/Wswitch-outside-range-4.C
> > @@ -0,0 +1,9 @@
> > +// PR c++/90875
> > +// { dg-options "-Wno-pedantic -Wno-switch-outside-range" }
>
> (You can also use __extension__ so that you don't need -Wno-pedantic.)
>
> > +void f(char c)
> > +{
> > + switch (c)
> > +
> > +case -300 ... 300:; // { dg-bogus "lower value in case label range
> > less than minimum value for type|upper value in case label range exceeds
> > maximum value for type" }
> > +}
>
> The tests belong to c-c++-common/ because we want to test both the C and
> C++ compilers.
>
> Please also fix the formatting as Jakub suggested.
>
> Thanks,
> --
> Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-06-1
[C++ PATCH] PR c++/90875 - added -Wswitch-outside-range option.
This patch adds a new warning option: Wswitch-outside-range, so that
users are able to selectively control the warning. The warning is
enabled by default.
Best,
Matthew Beliveau
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-06-14 Matthew Beliveau
PR c++/90875 - added -Wswitch-outside-range option
* doc/invoke.texi (Wswitch-outside-range): Document.
* c-warn.c (c_do_switch_warnings): Implemented new Wswitch-outside-range
warning option.
* c.opt (Wswitch-outside-range): Added new option.
* g++.dg/warn/Wswitch-outside-range-1.C: New test.
* g++.dg/warn/Wswitch-outside-range-2.C: New test.
* g++.dg/warn/Wswitch-outside-range-3.C: New test.
* g++.dg/warn/Wswitch-outside-range-4.C: New test.
diff --git gcc/c-family/c-warn.c gcc/c-family/c-warn.c
index 5941c10cddb..b61694af638 100644
--- gcc/c-family/c-warn.c
+++ gcc/c-family/c-warn.c
@@ -1460,8 +1460,9 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
min_value) >= 0)
{
location_t loc = EXPR_LOCATION ((tree) node->value);
- warning_at (loc, 0, "lower value in case label range"
- " less than minimum value for type");
+ warning_at (loc, OPT_Wswitch_outside_range, "lower value in case"
+ " label range less than minimum value"
+ " for type");
CASE_LOW ((tree) node->value) = convert (TREE_TYPE (cond),
min_value);
node->key = (splay_tree_key) CASE_LOW ((tree) node->value);
@@ -1474,8 +1475,8 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
if (node == NULL || !node->key)
break;
location_t loc = EXPR_LOCATION ((tree) node->value);
- warning_at (loc, 0, "case label value is less than minimum "
- "value for type");
+ warning_at (loc, OPT_Wswitch_outside_range, "case label value is"
+ " less than minimum value for type");
splay_tree_remove (cases, node->key);
}
while (1);
@@ -1491,8 +1492,8 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
max_value) > 0)
{
location_t loc = EXPR_LOCATION ((tree) node->value);
- warning_at (loc, 0, "upper value in case label range"
- " exceeds maximum value for type");
+ warning_at (loc, OPT_Wswitch_outside_range, "upper value in case"
+ " label range exceeds maximum value for type");
CASE_HIGH ((tree) node->value)
= convert (TREE_TYPE (cond), max_value);
outside_range_p = true;
@@ -1503,7 +1504,7 @@ c_do_switch_warnings (splay_tree cases, location_t switch_location,
!= NULL)
{
location_t loc = EXPR_LOCATION ((tree) node->value);
- warning_at (loc, 0,
+ warning_at (loc, OPT_Wswitch_outside_range,
"case label value exceeds maximum value for type");
splay_tree_remove (cases, node->key);
outside_range_p = true;
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 572cf186262..4ced767ca99 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -819,6 +819,10 @@ Wswitch-bool
C ObjC C++ ObjC++ Var(warn_switch_bool) Warning Init(1)
Warn about switches with boolean controlling expression.
+Wswitch-outside-range
+C++ ObjC++ Var(warn_switch_outside_range) Warning Init(1)
+Warn about switch values that are outside of their type's range.
+
Wtemplates
C++ ObjC++ Var(warn_templates) Warning
Warn on primary template declaration.
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index bf9da0f0a6e..c23496b2668 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -5390,6 +5390,12 @@ switch ((int) (a == 4))
@end smallexample
This warning is enabled by default for C and C++ programs.
+@item -Wswitch-outside-range
+@opindex Wswitch-outside-range
+@opindex Wno-switch-outside-range
+Warn whenever a @code{switch} state has a value that is outside of it's
+respective type range.
+
@item -Wswitch-unreachable
@opindex Wswitch-unreachable
@opindex Wno-switch-unreachable
diff --git gcc/testsuite/g++.dg/warn/Wswitch-outside-range-1.C gcc/testsuite/g++.dg/warn/Wswitch-outside-range-1.C
new file mode 100644
index 000..29e56f3ba2d
--- /dev/null
+++ gcc/testsuite/g++.dg/warn/Wswitch-outside-range-1.C
@@ -0,0 +1,8 @@
+// PR c++/90875
+
+void f(char c)
+{
+ switch (c)
+case 300: // { dg-warning "case label value exceeds maximum value for type" }
+case -300:; // { dg-warning "case label value is less than minimum value for type" }
+}
diff --git gcc/testsuite/g++.dg/warn/Wswitch-outside-range-2.C gcc/testsuite/g++.dg/warn/Wswitch-outside-range-2.C
new file mode 100644
index 000..20cc019b209
--- /dev/null
+++ gcc/testsuite/g++.dg/warn/Wswitch-outside-range-2.C
@@ -0,0 +1,9 @@
+// PR c++/90875
+// { dg-options -Wno-switch-outside-range }
+
+void f(char c)
+{
+ switch (c)
+case 300: //{ dg-bogus "case labe
Re: MAINTAINERS (Write After Approval): Add myself.
Oops! Sorry about that, should be fixed now! On Tue, Jun 11, 2019 at 3:32 PM Rainer Orth wrote: > > Matthew Beliveau writes: > > > ChangeLog: > > > > 2019-06-11 Matthew Beliveau > > > I don't believe this ;-) > > > * MAINTAINERS: Add myself. > > > > --- MAINTAINERS (revision 272167) > > +++ MAINTAINERS (working copy) > > @@ -316,6 +316,7 @@ > > Gergö Barany > > Charles Baylis > > Tejas Belagod > > +Matthew Beliveau > > Jon Beniston > > Andrew Bennett > > Andrew Benson > > -- > - > Rainer Orth, Center for Biotechnology, Bielefeld University
MAINTAINERS (Write After Approval): Add myself.
ChangeLog: 2019-06-11 Matthew Beliveau * MAINTAINERS: Add myself. --- MAINTAINERS (revision 272167) +++ MAINTAINERS (working copy) @@ -316,6 +316,7 @@ Gergö Barany Charles Baylis Tejas Belagod +Matthew Beliveau Jon Beniston Andrew Bennett Andrew Benson
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
Hello,
Correct me if I'm wrong, but before the function checks for virtual
bases, an if-statement checks for extra_warnings on line 6049(when my
patch is applied). The check was there before I made my changes, and
my test fails
without -Wextra. Below is the code above the warning call:
if (extra_warnings)
for (vbases = CLASSTYPE_VBASECLASSES (t), i = 0;
vec_safe_iterate (vbases, i, ); i++)
{
basetype = BINFO_TYPE (binfo);
Unless I'm mistaken and you meant for me to check for extra_warnings
again in the if-statement directly above the warning call.
Thank you,
Matthew Beliveau
On Tue, Jun 11, 2019 at 12:43 AM Jason Merrill wrote:
>
> On 6/10/19 12:02 PM, Matthew Beliveau wrote:
> > if (!uniquely_derived_from_p (basetype, t))
> > - warning (OPT_Wextra, "virtual base %qT inaccessible in %qT due "
> > -"to ambiguity", basetype, t);
> > + warning (OPT_Winaccessible_base, "virtual base %qT inaccessible in "
> > +"%qT due to ambiguity", basetype, t);
>
> You mentioned using -Wextra for this message, and you have a testcase
> for it, but here you remove the only connection between this message and
> -Wextra: with this patch, the virtual base warning will be enabled by
> default. Perhaps you want to check extra_warnings in the if condition?
>
> Jason
[C++ PATCH] Added test for c++/87250
My test is the result of reducing a file with just "include ",
just like in the PR.
It was fixed in r269059 which added a similar test, except mine does
not use inheritance and is a bit simpler. So I figured it wouldn't
hurt to cover more scenarios for this ICE.
I ran the test in my build directory and received 4 passes and 1
unsupported test, which was expected.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-06-10 Matthew Beliveau
PR c++/87250
* g++.dg/cpp0x/pr87250.C: New test.
diff --git gcc/testsuite/g++.dg/cpp0x/pr87250.C gcc/testsuite/g++.dg/cpp0x/pr87250.C
new file mode 100644
index 000..fe4bf1407de
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/pr87250.C
@@ -0,0 +1,12 @@
+// PR c++/87250
+// { dg-do compile { target c++11 } }
+// { dg-options "-Os -fsyntax-only" }
+
+template struct a {
+ constexpr a(int) {}
+};
+template struct atomic;
+template <> struct atomic {
+ a b;
+ constexpr atomic(bool c) : b(c) {}
+};
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
Hello,
i changed the doc/invoke.texi like you suggested. Let me know If I
missed anything!
Thank you,
Matthew Beliveau
On Mon, Jun 10, 2019 at 11:39 AM Martin Sebor wrote:
>
> On 6/7/19 2:10 PM, Matthew Beliveau wrote:
> > This patch adds a new warning option: Winaccessible-base, so that
> > users are able to selectively control the warning. The warning is
> > enabled by default; however, for the virtual bases' warning, it only
> > triggers with -Wextra flag.
>
> With few exceptions the rest of the manual tends to refer to base
> classes. I would suggest to do the same in this update as well:
>
> @@ -4800,6 +4801,22 @@ is only active when
> @option{-fdelete-null-pointer-checks} is active,
> which is enabled by optimizations in most targets. The precision of
> the warnings depends on the optimization options used.
>
> +@item -Winaccessible-base @r{(C++, Objective-C++ only)}
> +@opindex Winaccessible-base
> +@opindex Wno-inaccessible-base
> +Warn when a base is inaccessible in derived due to ambiguity. The
> warning is
> +enabled by default. Note the warning for virtual bases is enabled by the
> +@option{-Wextra} option.
>
>
> I.e.,
>
>Warn when a base class is inaccessible in a class derived from
>it due to ambiguity.
>
> Martin
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-06-10 Matthew Beliveau
PR c++/90449 - add -Winaccessible-base option.
* doc/invoke.texi (Winaccessible-base): Document.
* c.opt (Winaccessible-base): Added new option.
* class.c (warn_about_ambiguous_bases): Changed name to:
maybe_warn_about_inaccessible_bases.
(maybe_warn_about_inaccessible_bases): Implemented new
Winaccessible-base warning option for both direct and virtual
base warnings.
(layout_class_type): Call to warn_about_ambiguous_bases changed to fit
new name.
* g++.dg/warn/Winaccessible-base-1.C: New file.
* g++.dg/warn/Winaccessible-base-2.C: New file.
* g++.dg/warn/Winaccessible-virtual-base-1.C: New file.
* g++.dg/warn/Winaccessible-virtual-base-2.C: New file.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 046d489f7eb..144f6da15d6 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -625,6 +625,10 @@ Wignored-attributes
C C++ Var(warn_ignored_attributes) Init(1) Warning
Warn whenever attributes are ignored.
+Winaccessible-base
+C++ ObjC++ Var(warn_inaccessible_base) Init(1) Warning
+Warn when a base is inaccessible in derived due to ambiguity.
+
Wincompatible-pointer-types
C ObjC Var(warn_incompatible_pointer_types) Init(1) Warning
Warn when there is a conversion between pointers that have incompatible types.
diff --git gcc/cp/class.c gcc/cp/class.c
index a2585a61f96..9884cedc997 100644
--- gcc/cp/class.c
+++ gcc/cp/class.c
@@ -199,7 +199,7 @@ static int walk_subobject_offsets (tree, subobject_offset_fn,
static int layout_conflict_p (tree, tree, splay_tree, int);
static int splay_tree_compare_integer_csts (splay_tree_key k1,
splay_tree_key k2);
-static void warn_about_ambiguous_bases (tree);
+static void maybe_warn_about_inaccessible_bases (tree);
static bool type_requires_array_cookie (tree);
static bool base_derived_from (tree, tree);
static int empty_base_at_nonzero_offset_p (tree, tree, splay_tree);
@@ -6017,7 +6017,7 @@ end_of_class (tree t, bool include_virtuals_p)
subobjects of U. */
static void
-warn_about_ambiguous_bases (tree t)
+maybe_warn_about_inaccessible_bases (tree t)
{
int i;
vec *vbases;
@@ -6025,6 +6025,10 @@ warn_about_ambiguous_bases (tree t)
tree binfo;
tree base_binfo;
+ /* If not checking for warning then return early. */
+ if (!warn_inaccessible_base)
+return;
+
/* If there are no repeated bases, nothing can be ambiguous. */
if (!CLASSTYPE_REPEATED_BASE_P (t))
return;
@@ -6036,8 +6040,8 @@ warn_about_ambiguous_bases (tree t)
basetype = BINFO_TYPE (base_binfo);
if (!uniquely_derived_from_p (basetype, t))
- warning (0, "direct base %qT inaccessible in %qT due to ambiguity",
- basetype, t);
+ warning (OPT_Winaccessible_base, "direct base %qT inaccessible "
+ "in %qT due to ambiguity", basetype, t);
}
/* Check for ambiguous virtual bases. */
@@ -6048,8 +6052,8 @@ warn_about_ambiguous_bases (tree t)
basetype = BINFO_TYPE (binfo);
if (!uniquely_derived_from_p (basetype, t))
- warning (OPT_Wextra, "virtual base %qT inaccessible in %qT due "
- "to ambiguity", basetype, t);
+ warning (OPT_Winaccessible_base, "virtual base %qT inaccessible in "
+ "%qT due to ambiguity", basetype, t);
}
}
@@ -6455,7 +6459,7 @@ layout_class_type (tree t, tree *virtuals_p)
error ("size of type %qT is too large (%qE bytes)", t, TYPE_SIZE_UNIT (t));
/* Warn about bases that c
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
Hello,
I've changed the name of warn_about_ambiguous_bases to
maybe_warn_about_inaccessible_bases.
I've attached the new patch below.
Best,
Matthew Beliveau
On Fri, Jun 7, 2019 at 4:42 PM Paolo Carlini wrote:
>
> Hi,
>
> On 07/06/19 22:31, Marek Polacek wrote:
> > On Fri, Jun 07, 2019 at 10:20:02PM +0200, Paolo Carlini wrote:
> >> Hi,
> >>
> >> On 07/06/19 22:10, Matthew Beliveau wrote:
> >>> @@ -6025,6 +6025,10 @@ warn_about_ambiguous_bases (tree t)
> >> Just a nit, but it seems weird to me that the function implementing
> >> warn_inaccessible_base is named warn_about_ambiguous_bases.
> > Maybe, but we want to keep the warning's name in sync with clang, and
> > renaming the function seems like unnecessary noise. I could go either
> > way.
>
> It's definitely a minor issue. But, given that we have a rationale for
> inaccessible_base - I didn't know that - I vote for renaming the
> function. A maybe_* name would be appropriate in that case, because the
> function doesn't always warn.
>
> Paolo.
>
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-06-10 Matthew Beliveau
PR c++/90449 - add -Winaccessible-base option.
* doc/invoke.texi (Winaccessible-base): Document.
* c.opt (Winaccessible-base): Added new option.
* class.c (warn_about_ambiguous_bases): Changed name to:
maybe_warn_about_inaccessible_bases.
(maybe_warn_about_inaccessible_bases): Implemented new
Winaccessible-base warning option for both direct and virtual
base warnings.
(layout_class_type): Call to warn_about_ambiguous_bases changed to fit
new name.
* g++.dg/warn/Winaccessible-base-1.C: New file.
* g++.dg/warn/Winaccessible-base-2.C: New file.
* g++.dg/warn/Winaccessible-virtual-base-1.C: New file.
* g++.dg/warn/Winaccessible-virtual-base-2.C: New file.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 046d489f7eb..144f6da15d6 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -625,6 +625,10 @@ Wignored-attributes
C C++ Var(warn_ignored_attributes) Init(1) Warning
Warn whenever attributes are ignored.
+Winaccessible-base
+C++ ObjC++ Var(warn_inaccessible_base) Init(1) Warning
+Warn when a base is inaccessible in derived due to ambiguity.
+
Wincompatible-pointer-types
C ObjC Var(warn_incompatible_pointer_types) Init(1) Warning
Warn when there is a conversion between pointers that have incompatible types.
diff --git gcc/cp/class.c gcc/cp/class.c
index a2585a61f96..9884cedc997 100644
--- gcc/cp/class.c
+++ gcc/cp/class.c
@@ -199,7 +199,7 @@ static int walk_subobject_offsets (tree, subobject_offset_fn,
static int layout_conflict_p (tree, tree, splay_tree, int);
static int splay_tree_compare_integer_csts (splay_tree_key k1,
splay_tree_key k2);
-static void warn_about_ambiguous_bases (tree);
+static void maybe_warn_about_inaccessible_bases (tree);
static bool type_requires_array_cookie (tree);
static bool base_derived_from (tree, tree);
static int empty_base_at_nonzero_offset_p (tree, tree, splay_tree);
@@ -6017,7 +6017,7 @@ end_of_class (tree t, bool include_virtuals_p)
subobjects of U. */
static void
-warn_about_ambiguous_bases (tree t)
+maybe_warn_about_inaccessible_bases (tree t)
{
int i;
vec *vbases;
@@ -6025,6 +6025,10 @@ warn_about_ambiguous_bases (tree t)
tree binfo;
tree base_binfo;
+ /* If not checking for warning then return early. */
+ if (!warn_inaccessible_base)
+return;
+
/* If there are no repeated bases, nothing can be ambiguous. */
if (!CLASSTYPE_REPEATED_BASE_P (t))
return;
@@ -6036,8 +6040,8 @@ warn_about_ambiguous_bases (tree t)
basetype = BINFO_TYPE (base_binfo);
if (!uniquely_derived_from_p (basetype, t))
- warning (0, "direct base %qT inaccessible in %qT due to ambiguity",
- basetype, t);
+ warning (OPT_Winaccessible_base, "direct base %qT inaccessible "
+ "in %qT due to ambiguity", basetype, t);
}
/* Check for ambiguous virtual bases. */
@@ -6048,8 +6052,8 @@ warn_about_ambiguous_bases (tree t)
basetype = BINFO_TYPE (binfo);
if (!uniquely_derived_from_p (basetype, t))
- warning (OPT_Wextra, "virtual base %qT inaccessible in %qT due "
- "to ambiguity", basetype, t);
+ warning (OPT_Winaccessible_base, "virtual base %qT inaccessible in "
+ "%qT due to ambiguity", basetype, t);
}
}
@@ -6455,7 +6459,7 @@ layout_class_type (tree t, tree *virtuals_p)
error ("size of type %qT is too large (%qE bytes)", t, TYPE_SIZE_UNIT (t));
/* Warn about bases that can't be talked about due to ambiguity. */
- warn_about_ambiguous_bases (t);
+ maybe_warn_about_inaccessible_bases (t);
/* Now that we're done with layout, give the base fields the real types. */
for (field = TYPE_FIE
[C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
This patch adds a new warning option: Winaccessible-base, so that
users are able to selectively control the warning. The warning is
enabled by default; however, for the virtual bases' warning, it only
triggers with -Wextra flag.
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-06-07 Matthew Beliveau
PR c++/90449 - add -Winaccessible-base option.
* doc/invoke.texi (Winaccessible-base): Document.
* c.opt (Winaccessible-base): Added new option.
* class.c (warn_about_ambiguous_bases): Implemented new
Winaccessible-base warning option for both direct and virtual
base warnings.
* g++.dg/warn/Winaccessible-base-1.C: New file.
* g++.dg/warn/Winaccessible-base-2.C: New file.
* g++.dg/warn/Winaccessible-virtual-base-1.C: New file.
* g++.dg/warn/Winaccessible-virtual-base-2.C: New file.
diff --git gcc/c-family/c.opt gcc/c-family/c.opt
index 046d489f7eb..144f6da15d6 100644
--- gcc/c-family/c.opt
+++ gcc/c-family/c.opt
@@ -625,6 +625,10 @@ Wignored-attributes
C C++ Var(warn_ignored_attributes) Init(1) Warning
Warn whenever attributes are ignored.
+Winaccessible-base
+C++ ObjC++ Var(warn_inaccessible_base) Init(1) Warning
+Warn when a base is inaccessible in derived due to ambiguity.
+
Wincompatible-pointer-types
C ObjC Var(warn_incompatible_pointer_types) Init(1) Warning
Warn when there is a conversion between pointers that have incompatible types.
diff --git gcc/cp/class.c gcc/cp/class.c
index a2585a61f96..0f26b34e38e 100644
--- gcc/cp/class.c
+++ gcc/cp/class.c
@@ -6025,6 +6025,10 @@ warn_about_ambiguous_bases (tree t)
tree binfo;
tree base_binfo;
+ /* If not checking for warning then return early. */
+ if (!warn_inaccessible_base)
+return;
+
/* If there are no repeated bases, nothing can be ambiguous. */
if (!CLASSTYPE_REPEATED_BASE_P (t))
return;
@@ -6036,8 +6040,8 @@ warn_about_ambiguous_bases (tree t)
basetype = BINFO_TYPE (base_binfo);
if (!uniquely_derived_from_p (basetype, t))
- warning (0, "direct base %qT inaccessible in %qT due to ambiguity",
- basetype, t);
+ warning (OPT_Winaccessible_base, "direct base %qT inaccessible "
+ "in %qT due to ambiguity", basetype, t);
}
/* Check for ambiguous virtual bases. */
@@ -6048,8 +6052,8 @@ warn_about_ambiguous_bases (tree t)
basetype = BINFO_TYPE (binfo);
if (!uniquely_derived_from_p (basetype, t))
- warning (OPT_Wextra, "virtual base %qT inaccessible in %qT due "
- "to ambiguity", basetype, t);
+ warning (OPT_Winaccessible_base, "virtual base %qT inaccessible in "
+ "%qT due to ambiguity", basetype, t);
}
}
diff --git gcc/doc/invoke.texi gcc/doc/invoke.texi
index 3e4f012b4fa..862ee794773 100644
--- gcc/doc/invoke.texi
+++ gcc/doc/invoke.texi
@@ -317,6 +317,7 @@ Objective-C and Objective-C++ Dialects}.
-Wignored-qualifiers -Wignored-attributes -Wincompatible-pointer-types @gol
-Wimplicit -Wimplicit-fallthrough -Wimplicit-fallthrough=@var{n} @gol
-Wimplicit-function-declaration -Wimplicit-int @gol
+-Winaccessible-base @gol
-Winit-self -Winline -Wno-int-conversion -Wint-in-bool-context @gol
-Wno-int-to-pointer-cast -Winvalid-memory-model -Wno-invalid-offsetof @gol
-Winvalid-pch -Wlarger-than=@var{byte-size} @gol
@@ -4800,6 +4801,22 @@ is only active when @option{-fdelete-null-pointer-checks} is active,
which is enabled by optimizations in most targets. The precision of
the warnings depends on the optimization options used.
+@item -Winaccessible-base @r{(C++, Objective-C++ only)}
+@opindex Winaccessible-base
+@opindex Wno-inaccessible-base
+Warn when a base is inaccessible in derived due to ambiguity. The warning is
+enabled by default. Note the warning for virtual bases is enabled by the
+@option{-Wextra} option.
+@smallexample
+@group
+struct A @{ int a; @};
+
+struct B : A @{ @};
+
+struct C : B, A @{ @};
+@end group
+@end smallexample
+
@item -Winit-self @r{(C, C++, Objective-C and Objective-C++ only)}
@opindex Winit-self
@opindex Wno-init-self
diff --git gcc/testsuite/g++.dg/warn/Winaccessible-base-1.C gcc/testsuite/g++.dg/warn/Winaccessible-base-1.C
new file mode 100644
index 000..2e32b0b119f
--- /dev/null
+++ gcc/testsuite/g++.dg/warn/Winaccessible-base-1.C
@@ -0,0 +1,7 @@
+// PR c++/90449
+
+struct A { int a; };
+
+struct B : A { };
+
+struct C : B, A { }; // { dg-warning "direct base 'A' inaccessible in 'C' due to ambiguity" }
diff --git gcc/testsuite/g++.dg/warn/Winaccessible-base-2.C gcc/testsuite/g++.dg/warn/Winaccessible-base-2.C
new file mode 100644
index 000..67bd740a763
--- /dev/null
+++ gcc/testsuite/g++.dg/warn/Winaccessible-base-2.C
@@ -0,0 +1,8 @@
+// PR c++/90449
+// { dg-options -Wno-inaccessible-base }
+
+struct A { int a; };
+
+struct B : A { };
+
+struct C : B, A { }; // { dg-bogus "direct base 'A' inaccessible in 'C' due to ambiguity"
