public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] libstdc++: remove an ignored qualifier on function return type
@ 2020-08-24 11:26 Krystian Kuźniarek
  2020-08-24 11:38 ` Jonathan Wakely
  0 siblings, 1 reply; 6+ messages in thread
From: Krystian Kuźniarek @ 2020-08-24 11:26 UTC (permalink / raw)
  To: gcc-patches, libstdc++

Hi,

First of all, sorry, I must have sent it as quoted-printable so spaces and
tabs are preserved.

A description of the problem/bug and how your patch addresses it:
I've got a small patch for -Wignored-qualifiers in system headers.

Testcases:
N/A, it's only a warning.

ChangeLog:
Sorry, contrib/mklog.py didn't quite work for me.
For some reason after instruction in line 129: "diff = PatchSet(data)" my
"diff" variable is always empty.

Bootstrapping and testing:
Tested that manually by recompling GCC, unfolding all headers with
`#include <stdc++.h>` and compiling what's been included by it.

The patch itself:

diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index eb3d6779205..e8fcb57a401 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -808,7 +808,7 @@ namespace __variant
 	{ using element_type = _Tp; };

       template <typename... _Args>
-	struct __untag_result<const void(*)(_Args...)>
+	struct __untag_result<void(*)(_Args...)>
 	: false_type
 	{ using element_type = void(*)(_Args...); };



Best regards,
Krystian

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: remove an ignored qualifier on function return type
  2020-08-24 11:26 [PATCH] libstdc++: remove an ignored qualifier on function return type Krystian Kuźniarek
@ 2020-08-24 11:38 ` Jonathan Wakely
  2020-08-28  6:55   ` Krystian Kuźniarek
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2020-08-24 11:38 UTC (permalink / raw)
  To: Krystian Kuźniarek; +Cc: gcc-patches, libstdc++

On 24/08/20 13:26 +0200, Krystian Kuźniarek via Libstdc++ wrote:
>Hi,
>
>First of all, sorry, I must have sent it as quoted-printable so spaces and
>tabs are preserved.
>
>A description of the problem/bug and how your patch addresses it:
>I've got a small patch for -Wignored-qualifiers in system headers.
>
>Testcases:
>N/A, it's only a warning.
>
>ChangeLog:
>Sorry, contrib/mklog.py didn't quite work for me.
>For some reason after instruction in line 129: "diff = PatchSet(data)" my
>"diff" variable is always empty.

So then you need to produce a changelog entry by hand.

>Bootstrapping and testing:
>Tested that manually by recompling GCC, unfolding all headers with
>`#include <stdc++.h>` and compiling what's been included by it.

That doesn't test this header at all.

What about the libstdc++ testsuite?

>The patch itself:
>
>diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
>index eb3d6779205..e8fcb57a401 100644
>--- a/libstdc++-v3/include/std/variant
>+++ b/libstdc++-v3/include/std/variant
>@@ -808,7 +808,7 @@ namespace __variant
> 	{ using element_type = _Tp; };
>
>       template <typename... _Args>
>-	struct __untag_result<const void(*)(_Args...)>
>+	struct __untag_result<void(*)(_Args...)>

I don't remember exactly why I put it there, but I seem to recall it
was necessary.


> 	: false_type
> 	{ using element_type = void(*)(_Args...); };
>
>
>
>Best regards,
>Krystian
>


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: remove an ignored qualifier on function return type
  2020-08-24 11:38 ` Jonathan Wakely
@ 2020-08-28  6:55   ` Krystian Kuźniarek
  2020-08-28  7:16     ` Krystian Kuźniarek
  2020-10-29 17:18     ` Jonathan Wakely
  0 siblings, 2 replies; 6+ messages in thread
From: Krystian Kuźniarek @ 2020-08-28  6:55 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches, libstdc++

> So then you need to produce a changelog entry by hand.
I had this problem on some old Ubuntu 18.04. Anyway, here's new ChangeLog:

    libstdc++-v3/ChangeLog:

            * include/std/variant: Fix -Wignored-qualifiers
            in system headers.


>That doesn't test this header at all.
It does but indirectly. What I meant by manual test was:
${GCC_GIT} -E contains_only_stdcpp_include.cpp > preprocessed.cpp
${GCC_GIT} -Wall -Wextra -pedantic -fsyntax-only preprocessed.cpp
By manipulating GCC_GIT variable to trunk GCC and patched GCC, I checked if
the warning is gone.

>What about the libstdc++ testsuite?
I hope you mean calling make bootstrap and make check. If that's ok, I
confirm it works on Manjaro and Ubuntu 18.04 with gcc10 and gcc8
respectively.

>I don't remember exactly why I put it there, but I seem to recall it
>was necessary.
I don't know your reasons but I can only tell that this patch seems to
compile and work just fine.

On Mon, 24 Aug 2020 at 13:38, Jonathan Wakely <jwakely@redhat.com> wrote:

> On 24/08/20 13:26 +0200, Krystian Kuźniarek via Libstdc++ wrote:
> >Hi,
> >
> >First of all, sorry, I must have sent it as quoted-printable so spaces and
> >tabs are preserved.
> >
> >A description of the problem/bug and how your patch addresses it:
> >I've got a small patch for -Wignored-qualifiers in system headers.
> >
> >Testcases:
> >N/A, it's only a warning.
> >
> >ChangeLog:
> >Sorry, contrib/mklog.py didn't quite work for me.
> >For some reason after instruction in line 129: "diff = PatchSet(data)" my
> >"diff" variable is always empty.
>
> So then you need to produce a changelog entry by hand.
>
> >Bootstrapping and testing:
> >Tested that manually by recompling GCC, unfolding all headers with
> >`#include <stdc++.h>` and compiling what's been included by it.
>
> That doesn't test this header at all.
>
> What about the libstdc++ testsuite?
>
> >The patch itself:
> >
> >diff --git a/libstdc++-v3/include/std/variant
> b/libstdc++-v3/include/std/variant
> >index eb3d6779205..e8fcb57a401 100644
> >--- a/libstdc++-v3/include/std/variant
> >+++ b/libstdc++-v3/include/std/variant
> >@@ -808,7 +808,7 @@ namespace __variant
> >       { using element_type = _Tp; };
> >
> >       template <typename... _Args>
> >-      struct __untag_result<const void(*)(_Args...)>
> >+      struct __untag_result<void(*)(_Args...)>
>
> I don't remember exactly why I put it there, but I seem to recall it
> was necessary.
>
>
> >       : false_type
> >       { using element_type = void(*)(_Args...); };
> >
> >
> >
> >Best regards,
> >Krystian
> >
>
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: remove an ignored qualifier on function return type
  2020-08-28  6:55   ` Krystian Kuźniarek
@ 2020-08-28  7:16     ` Krystian Kuźniarek
  2020-10-29 17:18     ` Jonathan Wakely
  1 sibling, 0 replies; 6+ messages in thread
From: Krystian Kuźniarek @ 2020-08-28  7:16 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches, libstdc++

Actually ChangeLog also uses tabs, so it should be:

libstdc++-v3/ChangeLog:

	* include/std/variant: Fix -Wignored-qualifiers
	in system headers.


On Fri, 28 Aug 2020 at 08:55, Krystian Kuźniarek <
krystian.kuzniarek@gmail.com> wrote:

> > So then you need to produce a changelog entry by hand.
> I had this problem on some old Ubuntu 18.04. Anyway, here's new ChangeLog:
>
>     libstdc++-v3/ChangeLog:
>
>             * include/std/variant: Fix -Wignored-qualifiers
>             in system headers.
>
>
> >That doesn't test this header at all.
> It does but indirectly. What I meant by manual test was:
> ${GCC_GIT} -E contains_only_stdcpp_include.cpp > preprocessed.cpp
> ${GCC_GIT} -Wall -Wextra -pedantic -fsyntax-only preprocessed.cpp
> By manipulating GCC_GIT variable to trunk GCC and patched GCC, I checked
> if the warning is gone.
>
> >What about the libstdc++ testsuite?
> I hope you mean calling make bootstrap and make check. If that's ok, I
> confirm it works on Manjaro and Ubuntu 18.04 with gcc10 and gcc8
> respectively.
>
> >I don't remember exactly why I put it there, but I seem to recall it
> >was necessary.
> I don't know your reasons but I can only tell that this patch seems to
> compile and work just fine.
>
> On Mon, 24 Aug 2020 at 13:38, Jonathan Wakely <jwakely@redhat.com> wrote:
>
>> On 24/08/20 13:26 +0200, Krystian Kuźniarek via Libstdc++ wrote:
>> >Hi,
>> >
>> >First of all, sorry, I must have sent it as quoted-printable so spaces
>> and
>> >tabs are preserved.
>> >
>> >A description of the problem/bug and how your patch addresses it:
>> >I've got a small patch for -Wignored-qualifiers in system headers.
>> >
>> >Testcases:
>> >N/A, it's only a warning.
>> >
>> >ChangeLog:
>> >Sorry, contrib/mklog.py didn't quite work for me.
>> >For some reason after instruction in line 129: "diff = PatchSet(data)" my
>> >"diff" variable is always empty.
>>
>> So then you need to produce a changelog entry by hand.
>>
>> >Bootstrapping and testing:
>> >Tested that manually by recompling GCC, unfolding all headers with
>> >`#include <stdc++.h>` and compiling what's been included by it.
>>
>> That doesn't test this header at all.
>>
>> What about the libstdc++ testsuite?
>>
>> >The patch itself:
>> >
>> >diff --git a/libstdc++-v3/include/std/variant
>> b/libstdc++-v3/include/std/variant
>> >index eb3d6779205..e8fcb57a401 100644
>> >--- a/libstdc++-v3/include/std/variant
>> >+++ b/libstdc++-v3/include/std/variant
>> >@@ -808,7 +808,7 @@ namespace __variant
>> >       { using element_type = _Tp; };
>> >
>> >       template <typename... _Args>
>> >-      struct __untag_result<const void(*)(_Args...)>
>> >+      struct __untag_result<void(*)(_Args...)>
>>
>> I don't remember exactly why I put it there, but I seem to recall it
>> was necessary.
>>
>>
>> >       : false_type
>> >       { using element_type = void(*)(_Args...); };
>> >
>> >
>> >
>> >Best regards,
>> >Krystian
>> >
>>
>>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: remove an ignored qualifier on function return type
  2020-08-28  6:55   ` Krystian Kuźniarek
  2020-08-28  7:16     ` Krystian Kuźniarek
@ 2020-10-29 17:18     ` Jonathan Wakely
  2020-10-31 12:18       ` Krystian Kuźniarek
  1 sibling, 1 reply; 6+ messages in thread
From: Jonathan Wakely @ 2020-10-29 17:18 UTC (permalink / raw)
  To: Krystian Kuźniarek; +Cc: Jonathan Wakely, libstdc++, gcc-patches

On Fri, 28 Aug 2020 at 07:56, Krystian Kuźniarek via Libstdc++
<libstdc++@gcc.gnu.org> wrote:
>
> > So then you need to produce a changelog entry by hand.
> I had this problem on some old Ubuntu 18.04. Anyway, here's new ChangeLog:
>
>     libstdc++-v3/ChangeLog:
>
>             * include/std/variant: Fix -Wignored-qualifiers
>             in system headers.
>
>
> >That doesn't test this header at all.
> It does but indirectly. What I meant by manual test was:
> ${GCC_GIT} -E contains_only_stdcpp_include.cpp > preprocessed.cpp
> ${GCC_GIT} -Wall -Wextra -pedantic -fsyntax-only preprocessed.cpp
> By manipulating GCC_GIT variable to trunk GCC and patched GCC, I checked if
> the warning is gone.
>
> >What about the libstdc++ testsuite?
> I hope you mean calling make bootstrap and make check. If that's ok, I
> confirm it works on Manjaro and Ubuntu 18.04 with gcc10 and gcc8
> respectively.
>
> >I don't remember exactly why I put it there, but I seem to recall it
> >was necessary.
> I don't know your reasons but I can only tell that this patch seems to
> compile and work just fine.

I see new test failures with that change:

include/variant:1039: error: invalid conversion from
'std::enable_if_t<true, const void> (*)(test02()::Visitor&&,
std::variant<int, double>&)' {aka 'void (*)(test02()::Visitor&&,
std::variant<int, double>&)'} to
'std::__detail::__variant::_Multi_array<co
nst void (*)(test02()::Visitor&&, std::variant<int,
double>&)>::__untag_result<const void (*)(test02()::Visitor&&,
std::variant<int, double>&)>::element_type' {aka 'const void
(*)(test02()::Visitor&&, std::variant<int, double>&)'} [-fpermissive]
UNRESOLVED: 20_util/variant/visit_r.cc compilation failed to produce executable


So I still think it's there for a reason.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] libstdc++: remove an ignored qualifier on function return type
  2020-10-29 17:18     ` Jonathan Wakely
@ 2020-10-31 12:18       ` Krystian Kuźniarek
  0 siblings, 0 replies; 6+ messages in thread
From: Krystian Kuźniarek @ 2020-10-31 12:18 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: Jonathan Wakely, libstdc++, gcc-patches

Yeah, that's true. Ok, it's not applicable then. Thanks!

On Thu, 29 Oct 2020 at 18:18, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:

> On Fri, 28 Aug 2020 at 07:56, Krystian Kuźniarek via Libstdc++
> <libstdc++@gcc.gnu.org> wrote:
> >
> > > So then you need to produce a changelog entry by hand.
> > I had this problem on some old Ubuntu 18.04. Anyway, here's new
> ChangeLog:
> >
> >     libstdc++-v3/ChangeLog:
> >
> >             * include/std/variant: Fix -Wignored-qualifiers
> >             in system headers.
> >
> >
> > >That doesn't test this header at all.
> > It does but indirectly. What I meant by manual test was:
> > ${GCC_GIT} -E contains_only_stdcpp_include.cpp > preprocessed.cpp
> > ${GCC_GIT} -Wall -Wextra -pedantic -fsyntax-only preprocessed.cpp
> > By manipulating GCC_GIT variable to trunk GCC and patched GCC, I checked
> if
> > the warning is gone.
> >
> > >What about the libstdc++ testsuite?
> > I hope you mean calling make bootstrap and make check. If that's ok, I
> > confirm it works on Manjaro and Ubuntu 18.04 with gcc10 and gcc8
> > respectively.
> >
> > >I don't remember exactly why I put it there, but I seem to recall it
> > >was necessary.
> > I don't know your reasons but I can only tell that this patch seems to
> > compile and work just fine.
>
> I see new test failures with that change:
>
> include/variant:1039: error: invalid conversion from
> 'std::enable_if_t<true, const void> (*)(test02()::Visitor&&,
> std::variant<int, double>&)' {aka 'void (*)(test02()::Visitor&&,
> std::variant<int, double>&)'} to
> 'std::__detail::__variant::_Multi_array<co
> nst void (*)(test02()::Visitor&&, std::variant<int,
> double>&)>::__untag_result<const void (*)(test02()::Visitor&&,
> std::variant<int, double>&)>::element_type' {aka 'const void
> (*)(test02()::Visitor&&, std::variant<int, double>&)'} [-fpermissive]
> UNRESOLVED: 20_util/variant/visit_r.cc compilation failed to produce
> executable
>
>
> So I still think it's there for a reason.
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-10-31 12:19 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-24 11:26 [PATCH] libstdc++: remove an ignored qualifier on function return type Krystian Kuźniarek
2020-08-24 11:38 ` Jonathan Wakely
2020-08-28  6:55   ` Krystian Kuźniarek
2020-08-28  7:16     ` Krystian Kuźniarek
2020-10-29 17:18     ` Jonathan Wakely
2020-10-31 12:18       ` Krystian Kuźniarek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).