Message ID | Y3Lea4Fo/Hl8iFNZ@redhat.com |
---|---|
State | New |
Headers | show |
Series | [v2] c++: Disable -Wignored-qualifiers for template args [PR107492] | expand |
On 11/14/22 14:33, Marek Polacek wrote: > On Thu, Nov 03, 2022 at 03:22:12PM -0400, Jason Merrill wrote: >> On 11/1/22 13:01, Marek Polacek wrote: >>> It seems wrong to issue a -Wignored-qualifiers warning for code like: >>> >>> static_assert(!is_same_v<void(*)(), const void(*)()>); >>> >>> because there the qualifier matters. Likewise in template >>> specialization: >>> >>> template<typename T> struct S { }; >>> template<> struct S<void(*)()> { }; >>> template<> struct S<const void(*)()> { }; // OK, not a redefinition >>> >>> I'm of the mind that we should disable the warning for template >>> arguments, as in the patch below. >> >> Hmm, I'm not sure why we would want to treat template arguments differently >> from other type-ids. Maybe only warn if funcdecl_p? > > I think that makes sense. There are other contexts in which cv-quals > matter, for instance trailing-return-type. Well, technically they matter in all contexts, including function declaration: const void f(); template <class T, class U> struct same; template <class T> struct same<T,T>{}; same<decltype(f),const void()> s; but much more likely to be a confused user in that case, whereas in a template context it's likely to be some deep magic. :) > Updated patch below, plus I've extended the testcase. Thanks, > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? OK. > -- >8 -- > It seems wrong to issue a -Wignored-qualifiers warning for code like: > > static_assert(!is_same_v<void(*)(), const void(*)()>); > > because there the qualifier matters. Likewise in template > specialization: > > template<typename T> struct S { }; > template<> struct S<void(*)()> { }; > template<> struct S<const void(*)()> { }; // OK, not a redefinition > > And likewise in other type-id contexts such as trailing-return-type: > > auto g() -> const void (*)(); > > This patch limits the warning to the function declaration context only. > > PR c++/107492 > > gcc/cp/ChangeLog: > > * decl.cc (grokdeclarator): Only emit a -Wignored-qualifiers warning > when funcdecl_p. > > gcc/testsuite/ChangeLog: > > * g++.dg/warn/Wignored-qualifiers3.C: New test. > --- > gcc/cp/decl.cc | 6 ++++- > .../g++.dg/warn/Wignored-qualifiers3.C | 24 +++++++++++++++++++ > 2 files changed, 29 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/warn/Wignored-qualifiers3.C > > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc > index 890cfcabd35..67b9f24d7d6 100644 > --- a/gcc/cp/decl.cc > +++ b/gcc/cp/decl.cc > @@ -13038,7 +13038,11 @@ grokdeclarator (const cp_declarator *declarator, > > if (type_quals != TYPE_UNQUALIFIED) > { > - if (SCALAR_TYPE_P (type) || VOID_TYPE_P (type)) > + /* It's wrong, for instance, to issue a -Wignored-qualifiers > + warning for > + static_assert(!is_same_v<void(*)(), const void(*)()>); > + because there the qualifier matters. */ > + if (funcdecl_p && (SCALAR_TYPE_P (type) || VOID_TYPE_P (type))) > warning_at (typespec_loc, OPT_Wignored_qualifiers, "type " > "qualifiers ignored on function return type"); > /* [dcl.fct] "A volatile-qualified return type is > diff --git a/gcc/testsuite/g++.dg/warn/Wignored-qualifiers3.C b/gcc/testsuite/g++.dg/warn/Wignored-qualifiers3.C > new file mode 100644 > index 00000000000..dedb38fc995 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/warn/Wignored-qualifiers3.C > @@ -0,0 +1,24 @@ > +// PR c++/107492 > +// { dg-do compile { target c++14 } } > +// { dg-additional-options "-Wignored-qualifiers" } > + > +// Here the 'const' matters, so don't warn. > +template<typename T> struct S { }; > +template<> struct S<void(*)()> { }; > +template<> struct S<const void(*)()> { }; // { dg-bogus "ignored" } > + > +template<typename T, typename U> constexpr bool is_same_v = false; > +template<typename T> constexpr bool is_same_v<T, T> = true; > + > +static_assert( ! is_same_v< void(*)(), const void(*)() >, ""); // { dg-bogus "ignored" } > + > +// Here the 'const' matters as well -> don't warn. > +auto g() -> const void (*)(); // { dg-bogus "ignored" } > +auto g() -> const void (*)() { return nullptr; } // { dg-bogus "ignored" } > + > +// Here as well. > +const void (*h)() = static_cast<const void (*)()>(h); // { dg-bogus "ignored" } > + > +// But let's keep the warning here. > +const void f(); // { dg-warning "ignored" } > +const void f() { } // { dg-warning "ignored" } > > base-commit: c41bbfcaf9d6ef5b57a7e89bba70b861c08a686b
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 890cfcabd35..67b9f24d7d6 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -13038,7 +13038,11 @@ grokdeclarator (const cp_declarator *declarator, if (type_quals != TYPE_UNQUALIFIED) { - if (SCALAR_TYPE_P (type) || VOID_TYPE_P (type)) + /* It's wrong, for instance, to issue a -Wignored-qualifiers + warning for + static_assert(!is_same_v<void(*)(), const void(*)()>); + because there the qualifier matters. */ + if (funcdecl_p && (SCALAR_TYPE_P (type) || VOID_TYPE_P (type))) warning_at (typespec_loc, OPT_Wignored_qualifiers, "type " "qualifiers ignored on function return type"); /* [dcl.fct] "A volatile-qualified return type is diff --git a/gcc/testsuite/g++.dg/warn/Wignored-qualifiers3.C b/gcc/testsuite/g++.dg/warn/Wignored-qualifiers3.C new file mode 100644 index 00000000000..dedb38fc995 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wignored-qualifiers3.C @@ -0,0 +1,24 @@ +// PR c++/107492 +// { dg-do compile { target c++14 } } +// { dg-additional-options "-Wignored-qualifiers" } + +// Here the 'const' matters, so don't warn. +template<typename T> struct S { }; +template<> struct S<void(*)()> { }; +template<> struct S<const void(*)()> { }; // { dg-bogus "ignored" } + +template<typename T, typename U> constexpr bool is_same_v = false; +template<typename T> constexpr bool is_same_v<T, T> = true; + +static_assert( ! is_same_v< void(*)(), const void(*)() >, ""); // { dg-bogus "ignored" } + +// Here the 'const' matters as well -> don't warn. +auto g() -> const void (*)(); // { dg-bogus "ignored" } +auto g() -> const void (*)() { return nullptr; } // { dg-bogus "ignored" } + +// Here as well. +const void (*h)() = static_cast<const void (*)()>(h); // { dg-bogus "ignored" } + +// But let's keep the warning here. +const void f(); // { dg-warning "ignored" } +const void f() { } // { dg-warning "ignored" }