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

[waffl3x <waffl3x@protonmail.com>]

> Any progress on this, or do I need to coax the process along?  :)

Yeah, I've been working on it since the copyright assignment process
has finished, originally I was going to note that on my next update
which I had hoped to finish today or tomorrow. Well, in truth I was
hoping to send one the same day that copyright assignment finished, but
I found a nasty bug so I spent all day adding test cases for all the
relevant overloadable operators. Currently, it crashes when calling a
subscript operator declared with an explicit object parameter in a
dependent context. I haven't looked into the fix yet, but I plan to.

Also, before I forget, what is the process for confirming my copyright
assignment on your end? Do you just need to check in with the FSF to
see if it went through? Let me know if there's anything you need from
me regarding that.

Aside from the bug that's currently present in the first patch, I only
have like 1 or 2 little things I want to change about it. I will make
those few changes to patch 1, finish patch 2 (diagnostics) which will
also include test cases for the new bug I found. After I am done that I
plan on adding the things that are missing, because I suspect that
looking into that will get me close to finding the fix for the crash.

> Hmm, is it? I see that clang thinks so, but I don't know where they get
> that idea from. The grammar certainly allows it:
> 
> > attribute-specifier-seqopt decl-specifier-seq declarator = initializer-clause
> 
> 
> and I don't see anything else that prohibits it.

You would be right for P0847R7, but CWG DR 2653 changed that. You can
find the updated grammar in dcl.fct section 3 (subsection? I'm not
really sure to be honest.)

I've also included a copy of the grammar here for your convenience.

https://eel.is/c++draft/dcl.fct#nt:parameter-declaration
parameter-declaration:
  attribute-specifier-seqopt thisopt decl-specifier-seq declarator
  attribute-specifier-seqopt decl-specifier-seq declarator = initializer-clause
  attribute-specifier-seqopt thisopt decl-specifier-seq abstract-declaratoropt
  attribute-specifier-seqopt decl-specifier-seq abstract-declaratoropt = initializer-clause 


> I was thinking to set a TREE_LANG_FLAG on the TREE_LIST node.

I did figure this is originally what you meant, and I can still change
it to go this route since I'm sure it's likely just as good. But I do
recall something I didn't like in the implementation that nudged me
towards using the purpose member instead. Either way, not a big deal. I
think I just liked not having to mess with a linked list as I am not
used to them as a data structure, it might have been that simple. :^)

I will try to get something done today, but I was struggling with
writing some of the tests, there's also a lot more of them now. I also
wrote a bunch of musings in comments that I would like feedback on.

My most concrete question is, how exactly should I be testing a
pedwarn, I want to test that I get the correct warning and error with
the separate flags, do I have to create two separate tests for each one?

I'm just going to include the little wall I wrote in decl.cc regarding
pedwarn, just in case I can't get this done tonight so I can get some
feedback regarding it. On the other hand, it might just not be very
relevant to this patch in particular as I kind of noted, but maybe
there's some way to do what I was musing about that I've overlooked. It
does end up a bit ranty I guess, hopefully that doesn't make it
confusing.

```
/* I believe we should make a switch for this feature specifically,
   I recall seeing discussion regarding enabling newer language
   features when set to older standards. I would advocate for a
   specific flag for each specific feature. Maybe they should all
   be under an override flag? -foverride-dialect=xobj,ifconstexpr (?)
   I dont think it makes sense to give each feature override it's own
   flag. I don't recall what the consensus was around this discussion
   either though.
   For the time being it's controlled by pedantic. I am concerned that
   tying this to pedantic going forward that one might want to enable
   -pedantic-errors while also enabling select features from newer
   dialects. It didn't look like this use case is supported to me.

   I suppose this will require redesign work to support, so for
   the purposes of this patch, emitting a pedwarn seems correct.
   I just don't like that it can't be suppressed on an individual
   basis.  */
if (xobj_parm && cxx_dialect < cxx23)
  pedwarn(DECL_SOURCE_LOCATION (xobj_parm), OPT_Wpedantic, "");
```

That's all for now, I will try, (but I am very much not promising,) to
have an update by the end of today (6-8 hours for me.) If I manage to
get that out, I will (finally) start moving forward on implementing the
missing and broken features, and the aforementioned bug.

Alex