[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" }
diff --git gcc/testsuite/g++.dg/warn/Winaccessible-virtual-base-1.C gcc/testsuite/g++.dg/wa
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
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. Paolo.
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
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. Marek
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
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.
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_FIELDS (t); field; field = DECL_CHAIN (field))
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 @@ Object
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
On Mon, Jun 10, 2019 at 10:19:51AM -0400, Matthew Beliveau wrote: > 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. Looks fine to me except the small issues with whitespaces in the CL entry. Marek
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
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
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 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
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
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
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, &binfo); 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
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
On 6/11/19 9:59 AM, Matthew Beliveau wrote: 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, &binfo); i++) Ah, yes, thanks, I should have looked at the wider context. The patch is OK. Jason
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
On Tue, Jun 11, 2019 at 10:33:07AM -0400, Jason Merrill wrote: > On 6/11/19 9:59 AM, Matthew Beliveau wrote: > > 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, &binfo); i++) > > Ah, yes, thanks, I should have looked at the wider context. The patch is > OK. Thanks! I checked in the patch. Marek
Re: [C++ PATCH] PR c++/90449 - add -Winaccessible-base option.
On 6/11/19 7:59 AM, Matthew Beliveau wrote:
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, &binfo); i++)
{
basetype = BINFO_TYPE (binfo);
Just as an FYI, testing warning flags prevents #pragma GCC
diagnostic from having the expected effect:
struct A { };
struct B : virtual A { };
struct C : A { };
struct D0 : B, C { }; // -Winaccessible-base (good)
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wextra"
struct D1 : B, C { }; // -Winaccessible-base not suppressed
#pragma GCC diagnostic pop
struct D2 : B, C { }; // -Winaccessible-base (good)
(A more realistic use case is supressing -Wextra before including
a third party header and then restoring it.)
This can be solved by adding a function (say, is_warning_enabled
(int)) that does the same #pragma-sensitive checking as warning()
and calling it instead of testing extra_warnings directly. I.e.,
if (is_diagnostic_enabled (OPT_Wextra))
...
warning (OPT_Winaccessible_base, ...);
Martin
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
