Re: [PATCH v7 1/1] c++: Initial support for P0847R7 (Deducing This) [PR102609]

[waffl3x <waffl3x@protonmail.com>]

On Wednesday, December 6th, 2023 at 1:48 AM, Jakub Jelinek <jakub@redhat.com> wrote:


> 
> 
> On Wed, Dec 06, 2023 at 07:33:21AM +0000, waffl3x wrote:
> 
> > Here is the next version, it feels very close to finished. As before, I
> > haven't ran a bootstrap or the full testsuite yet but I did run the
> > explicit-obj tests which completed as expected.
> > 
> > There's a few test cases that still need to be written but more tests
> > can always be added. The behavior added by CWG2789 works in at least
> > one case, but I have not added tests for it yet. The test cases for
> > dependent lambda expressions need to be fleshed out more, but a few
> > temporary ones are included to demonstrate that they do work and that
> > the crash is fixed. Explicit object conversion functions work, but I
> > need to add fleshed out tests for them, explicit-obj-basic5.C has that
> > test.
> > 
> > I'll start the tests now and report back if anything fails, I'm
> > confident everything will be fine though.
> > 
> > Alex
> 
> > From 937e12c57145bfd878a0bc4cd9735c2d3c4fcf22 Mon Sep 17 00:00:00 2001
> > From: Waffl3x waffl3x@protonmail.com
> > Date: Tue, 5 Dec 2023 23:16:01 -0700
> > Subject: [PATCH] P0847R7 (Deducing This) [PR102609] Another quick and dirty
> > patch for review, hopefully the last. gcc/cp/ChangeLog:
> 
> 
> Please add
> PR c++/102609
> line above this.
> 
> > * call.cc (build_this_conversion):
> 
> 
> Note, for the final submission, all the ):
> should be followed by descriptions what has changed in there (but not why).

Yeah, I remember the drill, it just takes me a long time so I've been
slacking.

> Plus it would be good to mention somewhere early in the cp/ChangeLog
> entry that the patch implements C++23 P0847R7 - Deducing this paper
> (unfortunately the ChangeLog verifier doesn't allow such free text above
> the ChangeLog entry where it used to be written some years ago,
> only allows there the PR line; I usually put such text after the ):
> of the first entry now and only after it write exactly what changed
> in that function. Does the patch also implement CWG2586?

Oh jeez, I had been doing it the way you're saying is rejected.
Shouldn't the ChangeLog verifier be changed to allow this?

The patch does not implement CWG2586 at this time. I couldn't determine
if it were ready to go or not. I have a skeleton of tests for it that I
never finished, but as far as I know the implementation does conform to
CWG2789, this just happened to be how it worked out.

> 
> Also, I don't see in the patch the expected
> gcc/c-family/
> * c-cppbuiltin.cc (c_cpp_builtins): Predefine
> __cpp_explicit_this_parameter=202110L for C++23.
> part plus gcc/testsuite/cpp{23,26}/feat-cxx*.C additions checking
> for that macro presence and its value.
> 
> Jakub

Yeah I was meaning to look into how to do that, I originally added the
test and then never included it in any of the patches, or that's what
remember anyway. This saves me the work though, I'll be sure to add
that.

Alex