On 10/18/23 07:46, waffl3x wrote:
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
Ah, yes, thanks.
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 wouldn't expect to need any actual dealing with the linked list, just
setting a flag in cp_parameter_declaration_list at the same point as the
existing PARENTHESIZED_LIST_P flag.
But given CWG2653 as you pointed out, your current approach is fine.
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?
Yes. I tend to use letter suffixes for tests that vary only in flags
(and expected results), e.g. feature1a.C, feature1b.C.
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, "");
Instead of OPT_Wpedantic, this should be controlled by
-Wc++23-extensions (OPT_Wc__23_extensions)
If you wanted, you could add a more specific warning option for this
(e.g. -Wc++23-explicit-this) which is also affected by
-Wc++23-extensions, but I would lean toward just using the existing
flag. Up to you.
Jason
