> > 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.
-- gcc/tree.h:530 #define FUNC_OR_METHOD_CHECK(T) TREE_CHECK2 (T, FUNCTION_TYPE, METHOD_TYPE) -- gcc/tree.h:2518 #define TYPE_METHOD_BASETYPE(NODE) \ (FUNC_OR_METHOD_CHECK (NODE)->type_non_common.maxval) The code doesn't seem to reflect that, perhaps since the underlying node type is the same (as far as I can tell, they both inherit from tree_type_non_common) it wasn't believed to be necessary. Upon looking at DECL_CONTEXT though I see it seems you were thinking of FUNCTION_DECL. I haven't observed DECL_CONTEXT being set for FUNCTION_DECL nodes though, perhaps it should be? Although it's more likely that it is being set and I just haven't noticed, but if that's the case I'll have to make a note to make sure it is being set for xobj member functions. I was going to say that this seems like a redundancy, but I realized that the type of a member function pointer is tied to the struct, so it actually ends up relevant for METHOD_TYPE nodes. I would hazard a guess that when forming member function pointers the FUNCTION_DECL node was not as easily accessed. If I remember correctly that is not the case right now so it might be worthwhile to refactor away from TYPE_METHOD_BASETYPE and replace uses of it with DECL_CONTEXT. I'm getting ahead of myself though, I'll stop here and avoid going on too much of a refactoring tangent. I do want this patch to make it into GCC14 after all. > > 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. Right, I'll try to remember to check this area in the future, but yeah that tracks because I did remove that flag. Removing that flag just so happened to be the start of this saga of bug fixes but alas, it had to be done. > > 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. I'll put a note for myself to look into it in the future, it's an oddity at minimum and oddities interest me :^). > > 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. Okay, makes sense, I'm mildly concerned about any possible side effects this might have since we have a function_type node suddenly going through execution paths that only method_type went through before. The whole "reference_to_this" "pointer_to_this" thing is a little confusing because I'm pretty sure that doesn't refer to the actual `this` object argument or parameter since I've seen it all over the place. Hopefully it's benign. > > 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. Unfortunately I don't have any tests for private/public access yet, it's one of the area's I've neglected. Unfortunately I probably won't put too much effort into writing TOO many more right now as it takes up a lot of my time. I still have to clean up the ones I currently have and I have a few I wanted to write that are not yet written. > > 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. Very much so, thank you! An update on the regression testing, I had one test fail comparing against commit a4d2b108cf234e7893322a32a7956ca24e283b05 (GCC13) and I'm not sure if I need to be concerned about it. libgomp.c++/../libgomp.c-c++-common/for-16.c execution test I'm going to be starting tests for my patch against trunk now, once that is finished I should be ready to format a patch for review. That's all for now, thanks again. Alex
