On 11/22/23 15:46, waffl3x wrote:
On Tuesday, November 21st, 2023 at 8:22 PM, Jason Merrill <ja...@redhat.com>
wrote:
On 11/21/23 08:04, waffl3x wrote:
/* Nonzero for FUNCTION_DECL means that this decl is a non-static
- member function. */
+ member function, use DECL_IOBJ_MEMBER_FUNC_P instead. */
#define DECL_NONSTATIC_MEMBER_FUNCTION_P(NODE) \
(TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE)
+/* Nonzero for FUNCTION_DECL means that this decl is an implicit object
+ member function. */
+#define DECL_IOBJ_MEMBER_FUNC_P(NODE) \
+ (TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE)
I was thinking to rename DECL_NONSTATIC_MEMBER_FUNCTION_P rather than
add a new, equivalent one. And then go through all the current uses of
the old macro to decide whether they mean IOBJ or OBJECT.
I figure it would be easier to make that transition if there's a clear
line between old versus new. To be clear, my intention is for the old
macro to be removed once all the uses of it are changed over to the new
macro. I can still remove it for the patch if you like but having both
and removing the old one later seems better to me.
Hmm, I think changing all the uses is a necessary part of this change.
I suppose it could happen before the main patch, if you'd prefer, but it
seems more straightforward to include it.
+ else if (declarator->declarator->kind == cdk_pointer)
+ error_at (DECL_SOURCE_LOCATION (xobj_parm),
+ /* "a pointer to function type cannot "? */
+ "a function pointer type cannot "
+ "have an explicit object parameter");
"pointer to function type", yes.
+ /* The locations being used here are probably not correct. */
Why not?
I threw them in just so I could call inform, but it doesn't feel like
the messages should be pointing at the parameter, but rather at the
full type declaration. When I put those locations in I wasn't sure how
to get the full declaration location, and I'm still not 100% confident
in how to do it, so I just threw them in and moved on.
That would be more precise, but I think it's actually preferable for the
inform to have the same location as the previous error to avoid
redundant quoting of the source.
Let's clear xobj_parm after giving an error in the TYPENAME case
I don't like the spirit of this very much, whats your reasoning for
this? We're nearly at the end of the scope where it is last used, I
think it would be more unclear if we suddenly set it to NULL_TREE near
the end. It raises the question of whether that assignment actually
does anything, or if we are just trying to indicate that it isn't being
used anymore, but I already made sure to declare it in the deepest
scope possible. That much should be sufficient for indicating it's
usage, no?
Hmm, I think I poked at that and changed my mind, but forgot to delete
the suggestion. Never mind.
if ((!methodp && !DECL_XOBJ_MEMBER_FUNC_P (decl))
|| DECL_STATIC_FUNCTION_P (decl))
I think this can just be if (DECL_OBJECT_MEMBER_FUNC_P (decl)).
Alright, and going forward I'll try to make more changes that are
consistent with this one. With that said I'm not sure it can, but I'll
take a close look and if you're right I'll make that change.
if (TREE_CODE (fntype) == METHOD_TYPE)
ctype = TYPE_METHOD_BASETYPE (fntype);
+ else if (DECL_XOBJ_MEMBER_FUNC_P (decl1))
+ ctype = DECL_CONTEXT (decl1);
All of this can be
if (DECL_CLASS_SCOPE_P (decl1))
ctype = DECL_CONTEXT (decl1);
I think I'm going to go ahead and clean that up now.
Sounds good to me, a lot of this stuff needs small cleanups and I'm
just concerned about making them too much.
My cleanup of the ctype logic is in now.
+ /* Error reporting here is a little awkward, if the type of the
+ object parameter is deduced, we should tell them the lambda
+ is effectively already const, or to make the param const if it is
+ not, but if it is deduced and taken by value shouldn't we say
+ that it's taken by copy and won't mutate?
+ Seems right to me, but it's a little strange. */
I think just omit the inform if dependent_type_p.
Maybe I don't understand what a dependent type is as well as I thought,
but doesn't this defeat every useful case? The most common being an
xobj parameter of lambda type, which will always be deduced. Unless a
template parameter does not count as a dependent type, which is not
something I've ever thought about before.
No, you're right. A template parameter is certainly dependent. I think
the informs are fine as they are.
Mildly related, a lot of the stuff I hacked together with multiple
levels of accessing macros and predicates was due to not being able to
find a solution for what I needed. I think we would highly benefit from
better documentation of the accessors and predicates. I believe I've
seen some that appear to be duplicates, and some where they don't
appear to be implemented properly or match their description. If there
is such a document please direct me to it as I have spent an hour or so
each time I stumble on one of these problems.
In the section I wrote in add_method I spent I think about 3 hours
trying to figure out what combination of predicates and accessing
macros was correct for what I was trying to do. It gets pretty
convoluted to traverse, especially when the implementations of them are
rather involved. Finding non_reference was a big help for example, and
I stumbled across it by pure luck.
If we don't have this kind of documentation anywhere yet, I'm happy to
work on it next. I've spent a lot of time looking at it all so I feel
like I have a DECENT grasp on some of it. I think it would greatly
enhance the status quo, and might even help us figure out what needs to
be cleaned up, removed, replaced, etc.
Side side note, if said document does exist already, I either need to
learn how to search the wiki more effectively or it needs some sort of
improvement.
There isn't really such a document; the comments are the main internals
documentation, such as it is. I wonder about reorganizing cp-tree.h
rather than creating a separate document that is likely to go out of
date more quickly?
Jason
