On 10/28/23 00:07, waffl3x wrote:
I wanted to change DECL_NONSTATIC_MEMBER_FUNCTION_P to include explicit
object member functions, but it had some problems when I made the
modification. I also noticed that it's used in cp-objcp-common.cc so
would making changes to it be a bad idea?

-- cp-tree.h
```
/* Nonzero for FUNCTION_DECL means that this decl is a non-static
    member function.  */
#define DECL_NONSTATIC_MEMBER_FUNCTION_P(NODE) \
   (TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE)
```
I didn't want to investigate the problems as I was knee deep in
investigating the addressof bug. So I instead modified
DECL_FUNCTION_MEMBER_P to include explicit object member functions and
moved on.

-- cp-tree.h
```
/* Nonzero for FUNCTION_DECL means that this decl is a member function
    (static or non-static).  */
#define DECL_FUNCTION_MEMBER_P(NODE) \
   (DECL_NONSTATIC_MEMBER_FUNCTION_P (NODE) || DECL_STATIC_FUNCTION_P (NODE) \
   || DECL_IS_XOBJ_MEMBER_FUNC (NODE))
```
I am mostly just mentioning this here in case it becomes more relevant
later. Looking at how much DECL_NONSTATIC_MEMBER_FUNCTION_P is used
throughout the code I now suspect that adding explicit object member
functions to it might cause xobj member functions to be treated as
regular member functions when they should not be.

If this change were to stick it would cause a discrepancy in the
behavior of DECL_NONSTATIC_MEMBER_FUNCTION_P and it's name. If we were
to do this, I think it's important we document the discrepancy and why
it exists, and in the future, it should possibly be refactored. One
option would be to simply rename it to DECL_IOBJ_MEMBER_FUNCTION_P.
After all, I suspect that it's unlikely that the current macro
(DECL_NONSTATIC_MEMBER_FUNCTION_P) is being used in places that concern
explicit object member functions. So just adding explicit object member
functions to it will most likely just result in headaches.

It seems to me that would be the best solution, so when and if it comes
up again, I think that route should be considered.

Agreed, it sounds good to rename the current macro and then add a new macro that includes both implicit and explicit, assuming that's a useful category.

Secondly, there are some differences in the nodes describing an
explicit object member function from those describing static member
functions and implicit object member functions that I am not sure
should be present.

I did my best to summarize the differences, if you want the logs of
tree_debug that I derived them from I can provide them. Most of my
understanding of the layout of the nodes is from reading print-tree.cc
and looking at debug_tree outputs, so it's possible I made a mistake.

I am opting to use the names of members as they are output by
debug_tree, I recognize this is not always the actual name of the
member in the actual tree_node structures.

Additionally, some of the differences listed are to be expected and are
most likely the correct values for each node. However, I wanted to be
exhaustive when listing them just in case I am mistaken in my opinion
on whether the differences should or should not occur.

The following declarations were used as input to the compiler.
iobj decl:
struct S { void f() {} };
xobj decl:
struct S { void f(this S&) {} };
static decl:
struct S { static void f(S&) {} };

These differences can be observed in the return values of
grokdeclarator for each declaration.

1. function_decl::type::tree_code
iobj: method_type
xobj: function_type
stat: function_type
2. function_decl::type::method basetype
iobj: <record_type 0x7ffff7194c78 S>
xobj: NULL/no output
stat: NULL/no output
3. function_decl::type::arg-types::tree_list[0]::value
iobj: <pointer_type>
xobj: <reference_type>
stat: <reference_type>
4. function_decl::decl_6
iobj: false/no output
xobj: false/no output
stat: true
5. function_decl::align
iobj: 16
xobj: 8
stat: 8
6. function_decl::result::uid
iobj: D.2513
xobj: D.2513
stat: D.2512
7. function_decl::full-name
iobj: "void S::f()"
xobj: "void S::f(this S&)"

Differences 1, 3, and 7 seem obviously correct to me for all 3
declarations, 6 is a little bizarre to me, but since it's just a UID
it's merely an oddity, I doubt it is concerning.

Agreed.

That leaves 2, 4, and 5.

2. I am pretty sure xobj functions should have the struct they are a
part of recorded as the method basetype member. I have already checked
that function_type and method_type are the same node type under the
hood and it does appear to be, so it should be trivial to set it.
However I do have to wonder why static member functions don't set it,
it seems to be that it would be convenient to use the same field. Can
you provide any insight into that?

method basetype is only for METHOD_TYPE; if you want the containing type of the function, that's DECL_CONTEXT.

4. I have no comment here, but it does concern me since I don't
understand it at all.

In the list near the top of cp-tree.h, DECL_LANG_FLAG_6 for a FUNCTION_DECL is documented to be DECL_THIS_STATIC, which should only be set on the static member.

5. I am pretty sure this is fine for now, but if xobj member functions
ever were to support virtual/override capabilities, then it would be a
problem. Is my understanding correct, or is there some other reason
that iobj member functions have a different value here?

It is surprising that an iobj memfn would have a different DECL_ALIGN, but it shouldn't be a problem; the vtables only rely on alignment being at least 2.

There are also some differences in the arg param in
cp_build_addr_expr_1 that concerns me, but most of them are reflected
in the differences I have already noted. I had wanted to include these
differences as well but I have been spending too much time staring at
it, it's no longer productive. In short, the indirect_ref node for xobj
member functions has reference_to_this set, while iobj member functions
do not.

That's a result of difference 3.

The baselink binfo field has the private flag set for xobj
member functions, iobj member functions does not.

TREE_PRIVATE on a binfo is part of BINFO_ACCESS, which isn't a fixed value, but gets updated during member search. Perhaps the differences in consideration of conversion to a base led to it being set or cleared differently? I wouldn't worry too much about it unless you see differences in access control.

I've spent too much time on this write up, so I am calling it here, it
wasn't all a waste of time because half of what I was doing here are
things I was going to need to do anyway at least. I still feel like I
spent too much time on it. Hopefully it's of some value for me/others
later.

I hope my responses are helpful as well.

Jason

Reply via email to